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

"Group" is read when editor is focused #89230

Closed
isidorn opened this issue Jan 24, 2020 · 7 comments
Closed

"Group" is read when editor is focused #89230

isidorn opened this issue Jan 24, 2020 · 7 comments

Comments

@isidorn
Copy link
Contributor

@isidorn isidorn commented Jan 24, 2020

Turn on VoiceOver, focus an editor
The following is read
"x editor, Editor Group 1 group".

  1. group should not be read
  2. Editor Group 1 should only be read if there is more than one group

fyi @ivanfetch

@isidorn

This comment has been minimized.

Copy link
Contributor Author

@isidorn isidorn commented Jan 29, 2020

Could not fix 1), since "group" is automatically read by VoiceOver. Suggestions on how to fix this are welcome.
2) I fixed, fyi @bpasero on my approach

Adding bug so we get this verified.
To verify: turn on screen reader and focus an editor, make sure the "Editor Group" is only read out if there is more than 1 group.

@isidorn isidorn added the bug label Jan 29, 2020
@bpasero

This comment has been minimized.

Copy link
Member

@bpasero bpasero commented Jan 29, 2020

@isidorn can we have a debt item for Feb to fix it properly? I think we are happily setting this:

this.editorContainer.setAttribute('aria-label', this.computeAriaLabel());

But we actually do not update the editor options ariaLabel property at all. Neither from group changes nor from label changes. In fact I think we are computing the aria label twice: https://github.com/microsoft/vscode/blob/master/src/vs/workbench/browser/parts/editor/textEditor.ts#L97

@bpasero

This comment has been minimized.

Copy link
Member

@bpasero bpasero commented Jan 29, 2020

PS: I even wonder if we should go back and not update the aria label on each label change in untitled editors. After all that might be very confusing for users that need screen readers. We could simply just set the Untitled-<N> thing and it would probably be fine.

@isidorn

This comment has been minimized.

Copy link
Contributor Author

@isidorn isidorn commented Jan 29, 2020

@bpasero Yeah, I was just copying what you were doing on the label change. I will create a debt item

As for untitled editors, having the aria label Untitled <N> makes perfect sense for me. Feel free to change that.

sportshead added a commit to sportshead/vscode that referenced this issue Jan 30, 2020
* add setting for movable views

* untitled - do not compute label when first line change is >50 column

* Add style token for search editor input border. Fixes microsoft#89437.

* fix microsoft#89457

* Double click on file name to peek. Closes microsoft#89467

* fix microsoft#89502

* Remove redundant 'not forwarded' from ports view

Fixes microsoft/vscode-remote-release#2239

* Fix forward port action name in places it shouldn't contain 'a'

Fixes microsoft/vscode-remote-release#2241

* Untitled: show original resource name as description (fix microsoft#89423)

* ensure parent is checked when checking child, ensure child can be checked when parent is unchecked, microsoft#89500

* Fix enter keyboard shortcut on tunnels view

Fixes microsoft/vscode-remote-release#2221

* Editor navigation scrolled to top when opening the LRU item (fix microsoft#89519)

* No additional properties (fixes microsoft/vscode-remote-release#2222)

* Fix error in log with labeling port

Fixes microsoft/vscode-remote-release#2242

* Fix microsoft#89485

* fixes microsoft#89491

* misleading wording of "files.preventSaveConflicts" setting (fix microsoft#89510)

* add tree.hasElement()

related to microsoft#86201

* Don't remove trailing separator from drive letters (microsoft#89218)

Fixes microsoft/vscode-remote-release#1596

* fixes microsoft#88024

* fixes microsoft#86032

* fixes microsoft#89295

* Make ITheme.getTokenStyleMetadata return an object to be able to express "inherit" (microsoft#89600)

* use old uri for text edits that target renamed resource, microsoft#89506

* correct invalid checked states, e.g unchecked parent cannot have checked children, microsoft#89506, microsoft#89546, and microsoft#89500

* fix microsoft#89507

* fixes microsoft#89472

* make that there is always a focus element in the peek tree, microsoft#89557

* fix microsoft#89422

* fixes microsoft#83771

* improve inspect tokens hover

* always show diff editor, except for deletes, microsoft#89501

* accessibility: do not have editor group in aria label if only one group

fixes microsoft#89230

* Fix microsoft#89598

* Fix microsoft#89462

* Fix enablement event

* Fixes microsoft#89600: Allow that semantic tokens change only partially the underlying TM tokens

* Fix microsoft#89550

* fixes microsoft#89609

* End sockets used for port forwarding when tunnel is closed

Fixes microsoft/vscode-remote-release#2237

* Sort forwarded ports

Fixes microsoft/vscode-remote-release#2219

* Fixes microsoft#89459: Do not have "Command Palette..." in simple editor widgets

* Fix microsoft#89481

* Fix microsoft#89551

* Improve display of task pick string label

Fixes microsoft#89434

* Fix description of task in task quick open

Fixes microsoft#89441

* fixes microsoft#86602

* fix test failures

* Fixes microsoft#89552: Throw from the provider when semantic tokens cannot be computed and keep old semantic tokens if this happens

* Revert "Freeze uris before trying to resolveExternalUri"

This reverts commit e8793a8.

Fixes microsoft#89624

* Disable tests to unblock build

* fixes microsoft#87772

* Fix microsoft#89623

* Fix microsoft#89555

* Removed search.location from search.contribution.ts (microsoft#89585)

Co-authored-by: Check your git settings! <chris@chris-laptop>

* log requests to server

* update last sync data if it was not there

* update last sync state if does not exist

* Add command for opening workspace tasks and make consistent

Fixes microsoft#89465

* Fix microsoft#89599

* Fix microsoft#89138

* Add max length to search editor titles
Fixes microsoft#89466.

* Fix microsoft#89648

* Live-update search editor options. Fixes microsoft#89473.

* Allow `workbench.action.search.toggleQueryDetails` to toggle search editor. Fixes microsoft#89536.

* Fix microsoft#87441

* Fix microsoft#89493

* fixes microsoft#89478

* Fix typo in issue reporter string

* don't update overrides if not provided

* Begin token refresh sooner, microsoft#88577

* Re-enable tokens store tests

* Fixes microsoft#89540: Avoid document.activeElement when in shadow DOM

* Fixes microsoft#89573: Rename setting and possible values

* debug: update js-debug-nightly to "2020.1.38943" @ 2020-01-29T01:07:02.427Z

* Fix microsoft#88731

* Fixes issue with overlapping timeline message

* fixes microsoft#89488

* Persist highlights on restore and save. (Not on open)
Fixes microsoft#89477.

* Temporarily fix microsoft#89406 until microsoft#89267 is resolved.

* Deals with microsoft#89553 for now

* Remove unused function

* Remove console log

* Remove no longer required parameter

* fixes microsoft#89579

* Mark TextDocumentContentChangeEvent properties readonly

Follow up on microsoft#88310

Callers should never mutate these events (and after microsoft#88310, this will now produce an error since the result should be frozen)

* Skip failing tests

* Allows for js and ts specific refactoring pages

* fixes microsoft#89580

Co-authored-by: SteVen Batten <6561887+sbatten@users.noreply.github.com>
Co-authored-by: Benjamin Pasero <benjamin.pasero@gmail.com>
Co-authored-by: Jackson Kearl <jakearl@microsoft.com>
Co-authored-by: Johannes Rieken <johannes.rieken@gmail.com>
Co-authored-by: Alex Ross <alros@microsoft.com>
Co-authored-by: Christof Marti <chrmarti@microsoft.com>
Co-authored-by: Sandeep Somavarapu <sasomava@microsoft.com>
Co-authored-by: Isidor Nikolic <inikolic@microsoft.com>
Co-authored-by: João Moreno <mail@joaomoreno.com>
Co-authored-by: Alexandru Dima <alexdima@microsoft.com>
Co-authored-by: Martin Aeschlimann <martinae@microsoft.com>
Co-authored-by: Check your git settings! <chris@chris-laptop>
Co-authored-by: AlexStrNik <alex.str.nik@gmail.com>
Co-authored-by: Rachel Macfarlane <ramacfar@microsoft.com>
Co-authored-by: Connor Peet <connor@peet.io>
Co-authored-by: Pine <octref@gmail.com>
Co-authored-by: Eric Amodio <eamodio@gmail.com>
Co-authored-by: Matt Bierner <matb@microsoft.com>
@sandy081

This comment has been minimized.

Copy link
Member

@sandy081 sandy081 commented Jan 31, 2020

@isidorn Group 1 is not read when there are more than one group

image

@sandy081 sandy081 reopened this Jan 31, 2020
@isidorn isidorn modified the milestones: January 2020, February 2020 Jan 31, 2020
@isidorn

This comment has been minimized.

Copy link
Contributor Author

@isidorn isidorn commented Jan 31, 2020

We properly update the aria-label on the vscode side, however VoiceOver does not properly read the updated one.
However since we already have a debt item for this here #89621
I would prefer to keep this closed and as part of that debt work check if we can do something better

@isidorn isidorn closed this Jan 31, 2020
@isidorn

This comment has been minimized.

Copy link
Contributor Author

@isidorn isidorn commented Feb 27, 2020

Since @bpasero fixed this in the end, I have just verified it works fine, Thus adding verified label.

@isidorn isidorn added the verified label Feb 27, 2020
@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.