Cycle through reporting annotation target summaries with nvda+d#14480
Merged
Conversation
3a1b795 to
d62f8ac
Compare
See test results for failed build of commit 6244b4a766 |
michaelDCurran
left a comment
Member
There was a problem hiding this comment.
Generally this looks good. And I've also tested myself and found no issues so far.
However, I would probably recommend the second pr be completed for review before this one is merged, just in case the other one shows up issues missed when consuming this one.
Then both can be merged around the same time.
nvda+d
nvda+dnvda+d
8 tasks
michaelDCurran
approved these changes
Jan 4, 2023
seanbudd
added a commit
that referenced
this pull request
Jan 5, 2023
Stacked against #14480 Supersedes #14426 Summary of the issue: Currently, NVDA only announces the first annotation target, and only announces the first annotation summary. #14480 introduces the ability to cycle through multiple annotations targets. This PR introduces announcing the presence of multiple annotation targets. When multiple annotation targets are present with different roles, NVDA should announce them. Example: A footnote, a comment, and a no-role annotation are present NVDA should announce "has footnote, has comment, has details" to make it easier for the user to find the annotation they want to interact with. Description of user facing changes When the annotation feature is enabled, NVDA will now report all the unique annotation target roles. Description of development approach From the virtual buffer, exposed comma separated values for the roles of annotation targets via the "detailsRoles" property. Converted all aria-details functions to plural form (handling a collection rather than a single optional value). Removed usages of, and deprecated nvdaObject.hasDetails, nvdaObject.detailsSummary, nvdaObject.detailsRole These are replaced with new Annotations types
7 tasks
seanbudd
added a commit
that referenced
this pull request
Jan 13, 2023
…14526) Follow up of #14507, #14480 Summary of the issue: Firefox incorrectly throws an error when browsing annotated content with an unsupported role. Instead an unknown role should be reported: i.e. "has details". The typing for annotation data structures were incorrect. Description of user facing changes When browsing content in Firefox with an annotation with unsupported role, "has details" is reported instead of throwing an error. Description of development approach Update typing to be more accurate for annotation data structures, log and return None in the Firefox case to match Chromium behaviour. Use Tuples rather than generators for annotations.targets and annotations.roles. Drops the superfluous/unused annotations.summaries
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Supersedes #14389 and #14426.
Fixes #14360
Summary of the issue:
Before this PR, NVDA was only aware of the first annotation target.
NVDA announces the presence of an annotation target.
nvda+dis used to announce the summary of the first annotation target.This PR introduces base code to support multiple annotation targets, and enables cycling though reporting each summary.
Similarly, #14507 introduces reporting the presence of multiple annotation targets.
Description of user facing changes
nvda+dnow cycles through reporting each annotation target for an annotated item. For example reading the summary of each child comment.Description of development approach
Creates abstractions for the relationships between annotation origins and targets.
The global command to report a summary of an annotation target has been updated.
Testing strategy:
Tested using
nvda+don the annotations test document with Chrome and Firefox: https://mdn.github.io/html-examples/aria-annotations/Confirmed that each comment was reported for the first example of comments.
Tested with Chrome and Firefox
Known issues with pull request:
Max 10 targets are fetched, this should be dynamic, however there is a Chrome bug preventing this, requiring investigation.
https://crbug.com/1399184
Change log entries:
New features
Code Review Checklist: