nvaccess
Add ability to scroll automatically in braille
- Add ability to scroll automatically with braille
- Add ability to scroll automatically in braille
Link to issue number:
Fixes #18573
Summary of the issue:
Users may wish to scroll braille automatically.
Description of user facing changes:
- Added an unassigned command to toggle scrolling braille automatically.
- Added unasigned commands to decrease and increase the autoScroll rate.
- In the braille settings subpanel, added a slider to set the autoScroll rate, measured in cells/seconds.
Description of developer facing changes:
The BrailleHandler class has a new method named autoScroll with an enable parameter, to toggle braille automatic -scroll.
Description of development approach:
Added an autoScroll`` method in the BrailleHandlerclass to toggle automatic scroll. This will usewx.CallLaterto call theautoScrollForwardmethod. Code has been added to stop autoScroll when the caret reaches the end of a document (the cursor is not moved), when a message is displayed in braille, when a new object is handled, on secure screens, and when the session is locked, via the_clearAll` method.
Also, if automatic scroll is enabled, when scrolling back or forward, autoScroll will be disabled and enabled to reset the timer and scroll forward again.
Testing strategy:
Tested locally.
Known issues with pull request:
None.
Code Review Checklist:
- [x] Documentation:
- Change log entry
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
- [x] Testing:
- Unit tests
- System (end to end) tests
- Manual testing
- [x] UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
- [x] API is compatible with existing add-ons.
- [x] Security precautions taken.
Some displays have capacitive sensors that can detect when you reach the end of the line of Braille and optionally advance automatically, so supporting this use case is also worth thinking about.
CC @FelixGruetzmacher, @LeonarddeR.
For now, the default reading speed is 10 chars/sec.
This means that the speed would be dependant on the braille display size, but that doesn't seem to be the case in your code yet. I like the assumed characters per sec approach though. That would mean you can increase scrolling speed when a line only contains a few characters.
Leonard wrote:
I like the assumed characters per sec approach
I write the code for that approach, but later I removed it due to feedback from people who is concerned about ability to understand this concept, thinking for example in older people, or people with cognitive difficulties. Seems that the BrailleExtender add-on used the seconds approach, not chars/sec. We may create a combo box to decide between seconds or char/sec, or adding a checkbox asking if this should be proportional to the display size or the number of cells used. cc: @gerald-hartig since this may be controversial.
For now I'll try to create a combo box. I thought abou using the number of used cells, not the display size, too. But perhaps this produces an uncomfortable feeling if someone doesn't know if all relevant information has been read, if interval about scrollings is variable. So a combo box is more flexible than a checkbox, to add several options if desired. In the spanish community, where there are users testing this, a combo box has been suggested for this.
@SaschaCowley , I think that the failure of Pyright is not related to the code. Seems that the analysis is not performed due to something related to the cache.
Sorry @SaschaCowley , forget my comment about Pyright. I've analyzed the code locally and have fixed it (now no I don't get errors and the check here has passed).
I see that this has been labelled with ConceptApproved, so I'll continue working on this. I think that adding a combo box for cells/seconds and seconds can be confusing, since in one case the value increases the rate, and in the other one the delay is increased. So I'm thinking in adding just a value, but trying to reflect the conversion between cells/seconds to seconds deppending on the display size. I have found people who doesn't understand the concept of cells/seconds, at least the first time. But I think that it has more sense that jst seconds, since the same value can be applied regardless to the display size, for example if someone connects a different braille display.
I'm not satisfied with the approach of converting cells/seconds to seconds. I think that it would be better to add an explanatory documentation and, in the gui, show directly characters/seconds. In the long term, I think that it would be more understandable that showing a measure unit and changing a related one in the configuration.
@seanbudd , thanks for your review. I've seen just a suggestion related to the user guide, and I've applied it.
Thanks for your review, @SaschaCowley . About only braille users, when a message is shown in braille, scrolling is disabled, but I'll try to fix this.
@SaschaCowley , I think that I've addressed all your comments, and I've tested this locally.
FYI, in #12049, I have finally had to use integers in milliseconds rather than float number due to GUI issues (with wx).
Cyrille wrote:
I have finally had to use integers in milliseconds
I'm investigating several solutions, like using percentages or milliseconds, but I'm not satisfied with the user experience. I've tried to investigate floatspin, but seems that it's not easy to select the text of the text control. But in fact, incrementing one cell/sec may provide enough granularity. We are talking about cells/sec, not secs/cells. And anyway the timeout with the same increment in cells/sec will be increased in a different number of milliseconds, depending on the display sice. But I think that cells/sec is better that secs or ms, to get a timeout proportional to the display size. I'll fix things if I find any errors, and I'll wait for your answer befor moving forward.
@nvdaes - I'm becoming convinced a slider could be best here? 1-20, with 0.5 increments?
@seanbudd wrote:
I'm becoming convinced a slider could be best here? 1-20, with 0.5 increments?
I think that this is not easy to achieve, since SelLineSize method accepts just integer values. Anyway, I think that the granularity is enough. I've asked Copilot to draw the curve y=40/x. I have thought that x represents chars/sec, and y timeout in secs. Hope this helps as an illustration.
@seanbudd . Imagining the curve, I agreed with you about modularity, and I think that I've find a way to improve it with a slider. Please let me know if something else requires my attention.
If all check pass, I think that this is ready for review. I've updated the user guide to explain that we are using a slider, and that, when using gestures to change the scroll rate, instead of percentages, NVDA will report cells per second. I would prefer to have the ability to know how many cells per seconds are selected, not just a percentage, but since we need a slider, I think that this is a solution.
Some system tests are failing, but I don't think that this is related to this PR, so I'll marking this as ready for review.
I don't really like the slider here. It means that we have separate values reported in the settings GUI and when changing the auto scroll rate with a gesture. wx.SpinCtrlDouble allows floating-point values. It also seems to select all on focus, so I don't think we'd need to subclass it.
If we do stick with the slider, the label needs to change to remove the units, as they're visually useless and misleading based on NVDA's output.
@SaschaCowley , I love SpinCtrlDouble. Unfortunately, NVDA doesn't read the label. If you have suggestions, please let me know, or I'll try to investigate it.
@nvdaes Damn, I only played with it in a little test application with no label. I won't be able to look into it until after I return to work on the 5th of January. @seanbudd will have to decide what happens from NV Access's side in the meantime
For now, though I prefer a spin box, I'll use percentages instead of reporting explicitly cells per second, since I cannot make the SpinCtrlDouble to be accessible (the label is not reported). I've tried to use addLabelledControl, addItem, and using Copilot suggestions to set accessible name, but NVDA is not reporting the label when pressing tab, though the label can be read with the navigator objects. And I don't like to increase values using 500 milliseconds instead of 0.5 seconds. So, though for me sliders in general are less precisse, since we don't know the absolute value of percentages, for now I feel that a slider is the best solution. Perhaps later this (and other sliders) may be replaced be spins with support for floads.