Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Announce when new cursor is added #109918

Closed
isidorn opened this issue Nov 3, 2020 · 9 comments
Closed

Announce when new cursor is added #109918

isidorn opened this issue Nov 3, 2020 · 9 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues editor-multicursor Editor multiple cursor issues feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes on-testplan
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Nov 3, 2020

Currently multi cursors are not accessible.
For starters we shuold announce whenever a new cursor is added.

@alexdima let me know if you have a good code pointer

@isidorn isidorn added feature-request Request for new features or functionality accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues editor-multicursor Editor multiple cursor issues labels Nov 3, 2020
@isidorn isidorn added this to the November 2020 milestone Nov 3, 2020
@isidorn isidorn self-assigned this Nov 3, 2020
@isidorn isidorn modified the milestones: November 2020, On Deck Nov 30, 2020
@alexdima
Copy link
Member

alexdima commented Dec 4, 2020

@isidorn there are multiple ways to end up with multiple cursors. Which commands do you have in mind (ctrl+alt+up/down, column selection commands, ctrl+d, ctrl+shift+L)?

@isidorn
Copy link
Contributor Author

isidorn commented Dec 7, 2020

@alexdima I hoped there was a common layer that all these commands used and that we could announce in that layer.
Since I think the experience should be the same, no matter the command.

But if we have to do it on a command layer - then all commands that are keyboard triggered make good sense.

@alexdima
Copy link
Member

alexdima commented Dec 8, 2020

Here are the actions (their class names). They are living in multicursor.ts:

  • InsertCursorAbove
  • InsertCursorBelow
  • InsertCursorAtEndOfEachLineSelected
  • InsertCursorAtEndOfLineSelected
  • InsertCursorAtTopOfLineSelected
  • AddSelectionToNextFindMatchAction
  • AddSelectionToNextFindMatchAction
  • MoveSelectionToNextFindMatchAction
  • MoveSelectionToPreviousFindMatchAction
  • SelectHighlightsAction
  • CompatChangeAll

@isidorn
Copy link
Contributor Author

isidorn commented Dec 8, 2020

Thanks!
Assigning to this milestone so I take a look into this.

@isidorn isidorn modified the milestones: On Deck, December/January 2021 Dec 8, 2020
@isidorn isidorn modified the milestones: January 2021, February 2021 Jan 27, 2021
@isidorn isidorn modified the milestones: February 2021, March 2021 Feb 22, 2021
@isidorn isidorn closed this as completed in 223f4f8 Mar 5, 2021
@isidorn
Copy link
Contributor Author

isidorn commented Mar 5, 2021

I have pushed a change that we announce the cursors added after each of this commands in the format:
Cursor added LINE:COLUMN
If there are multiple cursors added we announce
Cursor added LINE_1:COLUMN_1, LINE_2:COLUMN_2... LINE_N:COLUMN_N

Once additional cursors are removed via escape I announce
Removed secondary cursors

@alexdima if you have time you can take a look at the code changes.
I basically get the cursor state before the operation and after and announce the difference.

I will share this with the community and once they try it out I can hopefully push some additional improvements.

fyi @SaschaCowley @webczat

@isidorn
Copy link
Contributor Author

isidorn commented Mar 8, 2021

Let's keep this issue open for additional feedback

@isidorn isidorn reopened this Mar 8, 2021
@SamKacer
Copy link

Haven't tried this feature, but I really love this idea. I was thinking something like this should exist for a long time, since very often I would accidently add multi cursors without knowing and end up messing up a file. sometimes forcing me to revert a large size chunk of work and start over.

another idea I have, but this would probably be more appropriate as a extension than a built-in feature, wuld be to make some kind of inoffensive sound play whenever typing with multiple cursors. this would be analogous to NVDA beeping when typing with shift pressed when capslock is on. just a reminder that multi cursors are there.

@isidorn
Copy link
Contributor Author

isidorn commented Mar 10, 2021

@SamKacer thanks a lot for providing feedback, if you get a chance try it out with VS Code insiders and let us know what you think about the changes we did https://code.visualstudio.com/insiders/

On the vscode side we try to not play any sounds, but to leave this up to the screen reader. So if there is some aria signal to make the screen reader produce a sound we could add it.
Also as you say an extension could tackle this.

@isidorn
Copy link
Contributor Author

isidorn commented Mar 11, 2021

Closing this issue as the initial planned work is done.
Once the community has feedback we can tackle that in separate issues.

Verifier: turn on screen reader, and use some command that adds additional cursors, make sure the screen reader reads the new cursor positions.

@isidorn isidorn closed this as completed Mar 11, 2021
@isidorn isidorn added verification-needed Verification of issue is requested and removed verification-needed Verification of issue is requested labels Mar 11, 2021
@isidorn isidorn added the on-release-notes Issue/pull request mentioned in release notes label Mar 26, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues editor-multicursor Editor multiple cursor issues feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes on-testplan
Projects
None yet
Development

No branches or pull requests

4 participants
@isidorn @alexdima @SamKacer and others