Create advanced option for experimental reporting of annotations#12409
Conversation
|
Hi |
It allows this work to be reviewed while the rest is worked on. |
|
Well, IMO, having a PR developing half of a feature does not help to do a good review. So here are already the points I can note; feel free to ignore it if you consider it is part of the other PR:
|
|
Thanks @CyrilleB79, I think this is all very useful PR feedback. I've updated the userGuide, this was actually unintentionally missed (still new here!). Soon, it can be updated to include the command when that piece of work is merged in. The feature here is a flag for developers and keen alpha users to opt-in for as rolling support is added. The userGuide may need to be updated further to reflect future work. |
| allowInChromium = integer(0, 3, default=0) | ||
|
|
||
| [annotations] | ||
| reportDetails = boolean(default=false) |
There was a problem hiding this comment.
For some experimental features I advise using an integer, so that we can have 3 way selections: (new behavior, old behavior, and default). This allows us to differentiate between those who have explicitly turned off a feature, and those who have just left it as the default. I think this situation is more important if we expect the flag to be long lived, and the default behavior to change.
For annotations I think a boolean is fine, since I hope we are able to remove the option once we are happy with the behavior. A bool allows us to turn it on to test, but most users won't be bothered by a half implementation.
Just to illustrate my first point. Imagine we want to do the following:
- Add a new feature, default to "off", behind a flag for dev purposes.
- Later when the implementation is complete, it stays configurable, but default to "on". We want all users to give it a try, but we are aware that it won't work for everyone.
Meanwhile:
- User A, aware of the early development, decides that feature is not for them. Chooses to disable it. We don't want to change their explicitly chosen preference.
- User B, not aware of the feature. They should try the feature when the default is changed to "on".
feerrenrut
left a comment
There was a problem hiding this comment.
Changes look good. It is a bit weird that this is not connected to anything. But given we are at the start of a dev cycle I don't think this is really a big deal. There is more ongoing here, and this allows it to be broken down more easily, the down side is minor.
Qchristensen
left a comment
There was a problem hiding this comment.
User Guide change looks good, thanks!
#12364 added a command to read the summary of the aria-details. To know whether or not an object has details, this should be reported. However, new users might not want to use this feature so #12409 added a config setting to opt-in. Description of how this pull request fixes the issue: Discards the "has details" state change message unless the annotations config option is turned on.
Link to issue number:
None
Summary of the issue:
If we are to start introducing experimental support for reporting annotations, it should be hidden behind an opt-in option in advanced preferences.
Description of how this pull request fixes the issue:
Adds a config setting in Advanced NVDA preferences to enable the discovery of "has details" in browseMode
Testing strategy:
Manual verification that the control is accessible both by mouse and tab navigation, and resets to default.
userGuideentry is shown when usingf1to view help for the settings group.Known issues with pull request:
This should wait until after alpha is merged to beta, as a featureless flag should not be released.
Change log entries:
None needed, when the feature is introduced the changelog item can be added
Code Review Checklist: