nvda icon indicating copy to clipboard operation
nvda copied to clipboard

Added mouse wheel rotation commands

Open cary-rowen opened this issue 3 months ago • 20 comments

Link to issue number:

Closes #15484

Summary of the issue:

Some dynamically loaded web pages and desktop applications, such as Dism++, only allow rotating via the hardware mouse wheel to display more content. NVDA can only interact with these through screen review mode. However, to load more items, mouse wheel usage is required.

Description of user facing changes

This pull request allows users to simulate mouse wheel rotation through gestures. Added four commands with no gestures assigned:

  • Rotates the mouse wheel up at the current mouse position
  • Rotates the mouse wheel down at the current mouse position
  • Rotates the mouse wheel left at the current mouse position
  • Rotates the mouse wheel right at the current mouse position

Description of development approach

I created a function within mouseHandler called rotateMouseWheel, which rotates the mouse wheel either vertically or horizontally, controlling the rotate direction and amount. For more information about mouse events, see WM_MOUSEWHEEL.

It's important to note that this function limits the rotating per execution to winUser.WHEEL_DELTA (120) or less. Although this PR performs the standard rotating amount of 120 each time, add-on developers might opt for a larger amount, as done by @beqabeqa473 in mouseWheelScrolling.

Testing strategy:

I tested the following cases:

Go to System Settings > Devices > Mouse > Roll the mouse wheel to scroll:

  • Multiple lines at a time
  • One screen at a time
  1. NVDA+Ctrl+V opens the voice settings panel and simulates mouse wheel rotating on slider controls such as speed and volume.
  2. Try simulating mouse wheel rotating in Microsoft Excel and visually verify that the scrolling is as expected.

Known issues with pull request:

Microsoft Excel correctly responds to system settings, whether "Multiple lines at a time" or "One screen at a time".

However, the slider in the NVDA voice settings panel always scrolls at the standard amount of 120 and does not respond to system settings, but is consistent with the behavior of physical mouse wheel.

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.

cary-rowen avatar Apr 30 '24 03:04 cary-rowen

Here are some things I'm not sure about yet:

  1. Due to #16419 I don't know where to put the change log entry.
  2. Is it necessary for scrolling gestures to play a beep or ui message when scrolling is performed to inform the user that the operation has been performed?

cary-rowen avatar Apr 30 '24 03:04 cary-rowen

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/4dxjbtw1b1jr5bx4/artifacts/output/nvda_snapshot_pr16462-31682,ac5a0ac7.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.1, INSTALL_END 1.1, BUILD_START 0.0, BUILD_END 30.2, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.5, FINISH_END 0.2

See test results for failed build of commit ac5a0ac7a4

AppVeyorBot avatar Apr 30 '24 03:04 AppVeyorBot

@cary-rowen nice feature, thanks very much for this great contribution. You write:

  1. Due to PR merge freeze until Monday May 6 AEDT #16419 I don't know where to put the change log entry.

I think 2024.2 will not take additional change log entris from alpha. But better we wait for NV Access confirmation on that.

  1. Is it necessary for scrolling gestures to play a beep or ui message when scrolling is performed to inform the user that the operation has been performed?

NVDA should read out what is under the mouse cursor, as it does when moving the mouse. Beeps are not an option in my opinion, because you don't have any clue where the mouse scrolled if NVDA doesn't read anything out.

Also we need to test if the review cursor is moved properly when it follows the mouse. You can enable this in the review cursor settings. Did you test this already?

Adriani90 avatar Apr 30 '24 04:04 Adriani90

Re the change log: You will have to put it in the correct section (2024.3) when it is created in alpha.

CyrilleB79 avatar Apr 30 '24 06:04 CyrilleB79

NVDA should read out what is under the mouse cursor, as it does when moving the mouse. Beeps are not an option in my opinion, because you don't have any clue where the mouse scrolled if NVDA doesn't read anything out.

Note that this is not the same behavior as actually scrolling the physical scroll wheel. It does not actually report new content under the mouse when the screen is scrolled.

hwf1324 avatar Apr 30 '24 07:04 hwf1324

Happy to see this feature!

I also would prefer that NVDA core avoid gestures that do not include the NVDA key.

For me, assigning these gestures are fine, if they include NVDA key also.

XLTechie avatar May 01 '24 03:05 XLTechie

Thanks to @LeonarddeR for the code review.

@LeonarddeR and @XLTechie

I assigned it a gesture with NVDA keys, NVDA+Windows+pageUp/Down hoping this would be more useful, I'd also like to hear what the community has to say.

Hi @Adriani90

NVDA should read out what is under the mouse cursor, as it does when moving the mouse.

This PR is intended to emulate the scroll wheel functionality on a physical mouse, which does not behave exactly like a mouse pointer.

@CyrilleB79

You will have to put it in the correct section (2024.3) when it is created in alpha.

No problem, I will do it.

cary-rowen avatar May 02 '24 06:05 cary-rowen

The need to scroll seems quite rare; personally, I would not have assigned it any keyboard shortcut.

CyrilleB79 avatar May 02 '24 09:05 CyrilleB79

Scrolling the wheel is probably rarer than moving the mouse pointer, I'd be willing to have the default gesture removed.

cary-rowen avatar May 05 '24 01:05 cary-rowen

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/n4pifls3r6ng06yi/artifacts/output/nvda_snapshot_pr16462-31725,20ccf788.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.0, INSTALL_END 1.0, BUILD_START 0.0, BUILD_END 28.0, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.4, FINISH_END 0.5

See test results for failed build of commit 20ccf7889e

AppVeyorBot avatar May 05 '24 01:05 AppVeyorBot

Can you please update the description to reflect the latest changes e.g. note that the gestures are unbound

seanbudd avatar May 06 '24 06:05 seanbudd

@Qchristensen an @seanbudd

I think there are two documentation-related questions here that need your confirmation. The first concerns whether commands to which gestures are not assigned should be documented in the user guide. I expressed my views in #13024.

The other one is the feature description, see @CyrilleB79' comment and my reply.

@seanbudd

Best, Cary

cary-rowen avatar May 09 '24 05:05 cary-rowen

@CyrilleB79 I changed the wording, would you mind reviewing it again?

cary-rowen avatar May 12 '24 23:05 cary-rowen

If @seanbudd approves this change, I will update the description of the PR.

cary-rowen avatar May 12 '24 23:05 cary-rowen

Thanks to @XLTechie for the English wording suggestion.

cary-rowen avatar May 14 '24 16:05 cary-rowen

@seanbudd I think this is ready.

cary-rowen avatar May 16 '24 13:05 cary-rowen