Improve restart confirmation dialog after changing interface language by AAClause · Pull Request #8537 · nvaccess/nvda

AAClause · 2018-07-23T01:44:32Z

Link to issue number:

Fixes #6416

Summary of the issue:

When we change language, the restart confirmation dialog is confusing. For more infos see #6416

Description of how this pull request fixes the issue:

This PR modify the dialog like this:

Text: "NVDA must be restarted for the new language to take effect."
buttons: "restart now" and "restart later"

Testing performed:

Tested with a try build. Success.

Known issues with pull request:

None

Change log entry:

Included in changes.t2t

josephsl

Hi,

A few things:

Could you mind merging in latest master branch commits please?
Gettext pot test is failing due to missing translator comments for restart dialog labels.
I think there's no need to use a separate variable for labels - you can just pass in the actual text.
I think one way you can improve the user experience might be to let the dialog take in the old and new language values (it's up to you as to whether you want to do this or not).
I think it might be best to keep the content of the dialog as close to the current alpha builds as possible - perhaps you can say, "For language change to take effect, you must restart NVDA. Select 'restart now' to restart NVDA and use the new language interface, or 'restart later' to use the new language laterr." - or something similar.

As for layout, it works and looks okay to me.

Thanks.

AAClause · 2018-07-23T07:28:18Z

Hi Joseph,
Thanks for your review. I'll fix all this.

I think one way you can improve the user experience might be to let the dialog take in the old and new language values (it's up to you as to whether you want to do this or not).

I think it might be best to keep the content of the dialog as close to the current alpha builds as possible - perhaps you can say, "For language change to take effect, you must restart NVDA. Select 'restart now' to restart NVDA and use the new language interface, or 'restart later' to use the new language laterr." - or something similar.

What do you think about:

NVDA must be restarted for the new language to take effect.
Select "restart now" to restart NVDA and use the interface in {newLanguage}, or "restart later" to continue use the interface in {currentLanguage} until next restart.

Thanks

josephsl · 2018-07-23T07:30:41Z

Hi, yes, that should be suffice for making the text more user friendly. Thanks. CC @LeonarddeR and @jcsteh in case they have comments for you.

josephsl · 2018-07-23T21:51:55Z

Hi,

Two more things:

When prmopted to restart, I think it might be helpful to bind "close" action to "restart later" - that is, pressing Escape key would mean user wants to restart later.
Can you document this change in the user guide under general/language section? You can say something like:

Note: you must restart NVDA to change user interface language. When the confirmation dialog appears, select "restart now" or "restart later" if you wish to use the new language now or at a later time, respectively.

Thanks.

josephsl · 2018-07-24T06:28:08Z

Hi,

There's a small wording issue (use -> using when talking about restart later text), although I think we don't have to worry about that until it takes flight on master snapshots. I'll take care of that change for you then.

Thanks for this hard work.

josephsl · 2018-07-24T06:29:40Z

Hi,

To @michaelDCurran, if it is okay with you, could I request a second look from you or Reef please? Thanks.

AAClause · 2018-07-24T07:14:37Z

Thanks Joseph! Sorry for my bad english.

josephsl · 2018-07-24T07:21:07Z

Hi, don’t worry about bad English (in contrary, your English is good). I’m more impressed with what you just committed –responding to small feedback. Thanks so much.

LeonarddeR

I agree with @josephsl that this looks quite good overall.

LeonarddeR

Oops, my previous review should've been a request for changes.

AAClause · 2018-07-31T06:29:57Z

Thanks @LeonarddeR, I was not confident about this...

derekriemer · 2018-08-05T06:37:37Z

@josephsl commented on Jul 23, 2018, 1:30 AM MDT:

Hi, yes, that should be suffice for making the text more user friendly. Thanks. CC @LeonarddeR and @jcsteh in case they have comments for you.

From: André-Abush Clause notifications@github.com
Sent: Monday, July 23, 2018 12:28 AM
To: nvaccess/nvda nvda@noreply.github.com
Cc: Joseph Lee joseph.lee22590@gmail.com; Comment comment@noreply.github.com
Subject: Re: [nvaccess/nvda] Improve restart confirmation dialog (#8537)

Hi Joseph,
Thanks for your review. I'll fix all this.

I think one way you can improve the user experience might be to let the dialog take in the old and new language values (it's up to you as to whether you want to do this or not).

I think it might be best to keep the content of the dialog as close to the current alpha builds as possible - perhaps you can say, "For language change to take effect, you must restart NVDA. Select 'restart now' to restart NVDA and use the new language interface, or 'restart later' to use the new language laterr." - or something similar.

What do you think about:

NVDA must be restarted for the new language to take effect.
Select "restart now" to restart NVDA and use the interface in {newLanguage}, or "restart later" to continue use the interface in {currentLanguage} until next restart.

Thanks

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub #8537 (comment) , or mute the thread https://github.com/notifications/unsubscribe-auth/AHgLkCK_cWjT5CV4XZojCaiM3WYwFaPoks5uJXsUgaJpZM4VaPzZ .

Select "restart now" to restart NVDA and use NVDA in {newLanguage}, or "restart later" to continue using NVDA in {currentLanguage} until the next restart.

jscholes · 2018-08-05T09:09:06Z

@derekriemer:

Select "restart now" to restart NVDA and use NVDA in {newLanguage}, or "restart later" to continue using NVDA in {currentLanguage} until the next restart.

Far too wordy and I don't think that the text needs to be over-complicated. The original issue (#6416) recommended the text:

NVDA must be restarted for the new language to take effect.

With modifications made to the OK and Cancel buttons to become Restart Now and Restart Later respectively. That seems sufficient, and nobody on the original thread raised any concerns.

Do people not feel that this makes the presence of the Restart Later button clear enough? If not, there are two options:

Modify the text in the current dialog as follows:

For the new language to take effect, the configuration must be saved and NVDA must be restarted.
Press Restart Now to save and restart NVDA, or Restart Later to manually save and exit at a later time.
Restart Now Restart Later

Change the wording completely and ask for confirmation instead:

For the new language to take effect, the configuration must be saved and NVDA must be restarted.
Do you want to save and restart NVDA now?
Yes No

I think option 1 is better, personally. Having said that I still think the short text from #6416 adequately gets the message across. The dialog text hasn't been modified since the issue was originally filed, and we already indicate that a restart is required in the label for the language dropdown:

Language (requires restart to fully take effect)

derekriemer · 2018-08-05T22:20:46Z

Far too wordy and I don't think that the text needs to be over-complicated. The original issue (#6416) recommended the text:

NVDA must be restarted for the new language to take effect.

With modifications made to the OK and Cancel buttons to become Restart Now and Restart Later respectively. That seems sufficient, and nobody on the original thread raised any concerns.

agree

feerrenrut

Generally this looks good! Could you please update the description on the pull request to include a change log entry.

Fix for nvaccess#6416

All concerns seem to have been addressed.

josephsl suggested changes Jul 23, 2018

View reviewed changes

AAClause force-pushed the i6416 branch from 5ab9c46 to d1fe117 Compare July 23, 2018 09:44

josephsl previously approved these changes Jul 24, 2018

View reviewed changes

josephsl changed the title ~~Improve restart confirmation dialog~~ Improve restart confirmation dialog after changing interface language Jul 24, 2018

AAClause dismissed josephsl’s stale review via 9d1dd5f July 24, 2018 06:58

josephsl previously approved these changes Jul 24, 2018

View reviewed changes

LeonarddeR approved these changes Jul 30, 2018

View reviewed changes

Comment thread source/gui/settingsDialogs.py Outdated

LeonarddeR previously requested changes Jul 30, 2018

View reviewed changes

AAClause dismissed josephsl’s stale review via 11cdf57 July 31, 2018 06:29

AAClause force-pushed the i6416 branch from dab8757 to 11cdf57 Compare July 31, 2018 06:29

AAClause force-pushed the i6416 branch from 11cdf57 to d06ed99 Compare August 5, 2018 11:28

AAClause force-pushed the i6416 branch 2 times, most recently from d6252b1 to 3cf0e83 Compare August 11, 2018 14:49

feerrenrut suggested changes Aug 15, 2018

View reviewed changes

Comment thread user_docs/en/userGuide.t2t Outdated

AAClause force-pushed the i6416 branch 2 times, most recently from 0f33d68 to 5c94c75 Compare August 15, 2018 12:10

AAClause added 3 commits August 21, 2018 09:59

Improve restart confirmation dialog after changing interface language

a2e6ad7

Fix for nvaccess#6416

Restore the original text

30fa767

Update changelog

86dbddb

userGuide.t2t: add a note about "restart later" button

8148eb8

AAClause force-pushed the i6416 branch from 5c94c75 to 8148eb8 Compare August 21, 2018 11:02

Merge branch 'master' into i6416

3e8d27e

feerrenrut approved these changes Dec 7, 2018

View reviewed changes

feerrenrut merged commit 45e03f4 into nvaccess:master Dec 7, 2018

nvaccessAuto added this to the 2018.4 milestone Dec 7, 2018

Uh oh!

Conversation

AAClause commented Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing performed:

Known issues with pull request:

Change log entry:

Uh oh!

josephsl left a comment

Choose a reason for hiding this comment

Uh oh!

AAClause commented Jul 23, 2018

Uh oh!

josephsl commented Jul 23, 2018 via email • edited by feerrenrut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josephsl commented Jul 23, 2018

Uh oh!

josephsl commented Jul 24, 2018

Uh oh!

josephsl commented Jul 24, 2018

Uh oh!

AAClause commented Jul 24, 2018

Uh oh!

josephsl commented Jul 24, 2018 via email • edited by feerrenrut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

AAClause commented Jul 31, 2018

Uh oh!

derekriemer commented Aug 5, 2018

Uh oh!

jscholes commented Aug 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

derekriemer commented Aug 5, 2018

Uh oh!

feerrenrut left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

AAClause commented Jul 23, 2018 •

edited

Loading

josephsl commented Jul 23, 2018 via email •

edited by feerrenrut

Loading

josephsl commented Jul 24, 2018 via email •

edited by feerrenrut

Loading

jscholes commented Aug 5, 2018 •

edited

Loading