nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
https://www.nvaccess.org/
Other
2.11k stars 637 forks source link

Settings panel: add a keyword argument in apply button handler #8315

Open josephsl opened 6 years ago

josephsl commented 6 years ago

Hi,

Related to #8310:

There are times when certain settings panels may wish to perform some operations when Apply button is pressed from Settings screen. For example, when UI language has changed and Apply button is pressed, NVDA should not restart.

Thus, a new keyword argument named "applyOnly" is hereby introduced to let panels take note of the difference between OK and Apply buttons. The immediate beneficiary is General category, in that when language has changed and Apply button is pressed, a warning should be shown and NVDA will not restart.

Thanks.

LeonarddeR commented 6 years ago

I tend to disagree on the behaviour of apply for the general category you propose. It makes sense to ask the user whether they want to restart NVDA for the changes to take effect, even if they press apply. In any case, any changes or addition of a new handler in this area should be left out of scope for NVDA 2018.2, unless it is only a bug fix.

josephsl commented 6 years ago

Hi, but what if, as noted in #8310, users are not given a chance to reassign keyboard modifiers? Thanks.

From: Leonard de Ruijter notifications@github.com Sent: Monday, May 21, 2018 10:30 AM To: nvaccess/nvda nvda@noreply.github.com Cc: Joseph Lee joseph.lee22590@gmail.com; Author author@noreply.github.com Subject: Re: [nvaccess/nvda] Settings panel: add a keyword argument in apply button handler (#8315)

I tend to disagree on the behaviour of apply for the general category you propose. It makes sense to ask the user whether they want to restart NVDA for the changes to take effect, even if they press apply. In any case, any changes or addition of a new handler in this area should be left out of scope for NVDA 2018.2

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nvaccess/nvda/issues/8315#issuecomment-390724872 , or mute the thread https://github.com/notifications/unsubscribe-auth/AHgLkFPM2S2RBjA5dJNtDTjWd-M4XFRrks5t0vm0gaJpZM4UHVtx .

LeonarddeR commented 6 years ago

I wouldn't do preSave, onSave and postSave sequentially for every panel, but first loop through all the panels and do preSave, than run all the onSave handlers, than all the postSave handlers. This keeps the user experience as tight to as it was before, i.e. if pre evaluation fails, nothing is saved. In the case of unchecking all modifier keys from the keyboard panel, pressing ok would just pop up the error message that tells one that at least one modifier has to be set. When dismissed, the settings dialog will return without saving anything. In the case of the general settings postSave, it will make sure that all settings are saved properly before restarting.

josephsl commented 6 years ago

Hi, yep, that could work. Thanks.

From: Leonard de Ruijter notifications@github.com Sent: Monday, May 21, 2018 10:40 AM To: nvaccess/nvda nvda@noreply.github.com Cc: Joseph Lee joseph.lee22590@gmail.com; Author author@noreply.github.com Subject: Re: [nvaccess/nvda] Settings panel: add a keyword argument in apply button handler (#8315)

I wouldn't do preSave, onSave and postSave sequentially for every panel, but first loop through all the panels and do preSave, than run all the onSave handlers, than all the postSave handlers. This keeps the user experience as tight to as it was before, i.e. if pre evaluation fails, nothing is saved. In the case of unchecking all modifier keys from the keyboard panel, pressing ok would just pop up the error message that tells one that at least one modifier has to be set. When dismissed, the settings dialog will return without saving anything. In the case of the general settings postSave, it will make sure that all settings are saved properly before restarting.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nvaccess/nvda/issues/8315#issuecomment-390727531 , or mute the thread https://github.com/notifications/unsubscribe-auth/AHgLkL0zQnbUwVpExMLA59GSXMEl8x1qks5t0vvggaJpZM4UHVtx .

Adriani90 commented 1 week ago

cc: @SaschaCowley