Skip to content

Add Keyboard layout setting to the Welcome dialog#6868

Merged
feerrenrut merged 3 commits into
nvaccess:masterfrom
dkager:i6863
May 30, 2017
Merged

Add Keyboard layout setting to the Welcome dialog#6868
feerrenrut merged 3 commits into
nvaccess:masterfrom
dkager:i6863

Conversation

@dkager

@dkager dkager commented Feb 10, 2017

Copy link
Copy Markdown
Contributor

For this addition I didn't rewrite the dialog to use guiHelper. As a consequence I'm not sure if it works out visually.
Fixes #6863.

Also:
* Remove empty line at the end of the detail message.
* Clean up translator comments.
* Add mnemonics to the settings controls.
Todo:
* Use guiHelper.
* Bring Keyboard settings dialog in line, i.e. add mnemonics there too.
@feerrenrut

Copy link
Copy Markdown
Contributor

I had a look at this to check the visual layout. Currently there is no gap (horizontally) between the label and wx.Choice. You can add a spacer for this. See guiHelper.py line 118. Also the vertical alignment of the choice control is wrong. It needs to be centred vertically. I think that this would be a good opportunity to convert this dialog to use the guiHelper.

@dkager

dkager commented Feb 10, 2017

Copy link
Copy Markdown
Contributor Author

Thank you. I'll move to guiHelper as part of this PR then.

@dkager

dkager commented Feb 10, 2017

Copy link
Copy Markdown
Contributor Author

Just sprinkled some guiHelper on this dialog.

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I have built this locally to visually inspect and ensure the spacing and alignment is ok.

Comment thread source/gui/__init__.py Outdated
index = self.kbdNames.index(config.conf["keyboard"]["keyboardLayout"])
self.kbdList.SetSelection(index)
except:
log.debugWarning("Could not set Keyboard layout list to current layout",exc_info=True)

@feerrenrut feerrenrut Apr 25, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log.debugWarning does not produce a sound in NVDA. If this is hit I would suggest that it is a serious enough error that users should be notified. I would propose changing this to log.error

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated my comment, debugWarning never produces a sound. error sounds are purposefully omitted from the release.

@dkager

dkager commented Apr 25, 2017

Copy link
Copy Markdown
Contributor Author

I copied the keyboard code from elsewhere. Arguably the welcome experience should be errorless from the user's perspective, even if under the hood everything's on fire. :)

@feerrenrut

Copy link
Copy Markdown
Contributor

Understood, this will be the case in a release. However pre-release we want to know about problems as early as possible to have a chance to fix them prior to the release and RC versions.

@dkager

dkager commented Apr 26, 2017

Copy link
Copy Markdown
Contributor Author

Do you suppose this also goes for the same log message when setting the layout from the settings dialog? That's where the code came from.

@dkager

dkager commented Apr 26, 2017

Copy link
Copy Markdown
Contributor Author

Made it an error for the welcome dialog. Didn't touch the settings dialog though.

feerrenrut added a commit that referenced this pull request Apr 27, 2017
Re issue: #6863
Merge branch 'dkager-i6863' into next
@feerrenrut feerrenrut merged commit 8737fa3 into nvaccess:master May 30, 2017
@nvaccessAuto nvaccessAuto added this to the 2017.3 milestone May 30, 2017
feerrenrut added a commit that referenced this pull request May 30, 2017
- PR #7169 : Editable div elements in Chrome are no longer have their label reported as their value while in browse mode. (Issue #7153)
- PR #6396 : An unbound gesture (script_restart) has been added to allow NVDA to be restarted quickly. (PR #6396)
- PR #6777 : A Braille setting has been added to "show messages indefinitely". (Issue #6669)
- PR #7133 : Pressing end while in browse mode of an empty Microsoft Word document no longer causes a runtime error. (Issue #7009)
- PR #6868 : The keyboard layout can now be set from the NVDA Welcome dialog. (Issue #6863)
- PR #6813 : The names of "landmarks" are abbreviated in Braille (Issue #3975)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants