Adjust wording of sign-in text by codeofdusk · Pull Request #10941 · nvaccess/nvda

codeofdusk · 2020-04-05T01:19:55Z

Link to issue number:

None.

Summary of the issue:

The text used to describe starting NVDA at logon differ in the welcome and settings dialogs.
Windows settings generally uses "sign-in" (with a hyphen), but the NVDA string reads "sign in".
It may be unclear to new users when settings apply (just for the current user or for everyone).

Description of how this pull request fixes the issue:

Updates the strings, modelling on Narrator's.

Testing performed:

None.

Known issues with pull request:

None.

Change log entry:

None.

AppVeyorBot · 2020-04-05T01:27:50Z

PR introduces Flake8 errors
Build (for testing PR)

See test results for failed build of commit 0c6ea584cd

codeofdusk · 2020-04-06T01:24:39Z

Cc @feerrenrut @LeonarddeR @michaelDCurran

michaelDCurran · 2020-04-06T03:14:45Z

I think the user guide will need to be updated also.

codeofdusk · 2020-04-06T03:15:47Z

There is no mention of those settings at all in the user guide, so no updates needed.

codeofdusk · 2020-04-06T03:16:43Z

Oh wait, it is just with broken formatting.

codeofdusk · 2020-04-06T03:50:28Z

OK, user guide now fixed.

michaelDCurran · 2020-04-06T05:04:47Z

I'd like to get your thoughts on this @Qchristensen .
I agree it is probably a good idea to use the term sign-in rather than logon as this is what Windows itself says these days. However, do you think changing this in NVDA and the user guide would have a major impact on our training material?

codeofdusk · 2020-04-06T05:42:52Z

NVDA already says "sign in" (in general settings) and "log on" in other places. This PR just makes things more consistent.

Qchristensen · 2020-04-06T06:52:41Z

I agree this is a useful change. Re the training material, I will update the Basic Training material. (I am intending to go through all the Basic Training material again soon. In the meantime, I will make an "addendum" to send to anyone who buys the audio or Braille versions for now).

It is also worth noting this change is in line with the Microsoft Style Guide, which advocates using "sign in" and "sign out": https://docs.microsoft.com/en-us/style-guide/a-z-word-list-term-collections/s/sign-in-sign-out

Interesting pickup @codeofdusk about the Windows settings hyphenating Sign-in, given their own style guide advises against that.

feerrenrut · 2020-04-06T09:59:00Z

 		# Windows (if checked, NVDA will start automatically after logging into Windows; if not, user must
 		# start NVDA by pressing the shortcut key (CTRL+Alt+N by default).
-		self.startAfterLogonCheckBox = wx.CheckBox(self, label=_("St&art NVDA after Windows sign in"))
+		self.startAfterLogonCheckBox = wx.CheckBox(self, label=_("St&art NVDA after Windows sign-in for me"))


We've just tried to shorten this. Instead, can you re-word this to:

`Start NVDA after I sign in"

This intentionally drops the word Windows. What else would this option be related to if not windows sign in. It can be clarified further in the user guide.

feerrenrut · 2020-04-06T10:03:18Z

-		self.startOnLogonScreenCheckBox = wx.CheckBox(self, label=_("Use NVDA on the Windows logon screen (requires administrator privileges)"))
+		self.startOnLogonScreenCheckBox = wx.CheckBox(
+			self,
+			label=_("Start NVDA before Windows sign-in for &everyone (requires administrator privileges)")


I don't think "for everyone" is necessary. The sign in screen is inherently for everyone.

Suggested change

label=_("Start NVDA before Windows sign-in for &everyone (requires administrator privileges)")

label=_("Use NVDA at Windows sign-in (requires administrator privileges)")

I also would like to drop the "requires administrator privileges" part. It would be better to explain why it can't be done when trying. For now it's probably enough to explain in the user guide.

feerrenrut · 2020-04-06T10:05:09Z

+		self.copySettingsButton = wx.Button(
+			self,
+			label=_(
+				"Use currently saved settings before sign-in and on secure screens (requires administrator privileges)"


The word "before" bothers me.

AppVeyorBot · 2020-04-22T00:35:51Z

Build (for testing PR)

See test results for failed build of commit b9480af717

codeofdusk · 2020-04-22T00:44:51Z

@feerrenrut PR updated.

AppVeyorBot · 2020-04-22T00:49:50Z

Build (for testing PR)

See test results for failed build of commit 6b7a90a9cd

feerrenrut · 2020-04-22T07:56:07Z

-		self.copySettingsButton= wx.Button(self, label=_("Use currently saved settings on the logon and other secure screens (requires administrator privileges)"))
+		self.copySettingsButton = wx.Button(
+			self,
+			label=_(


I suspect that the translator comment for this line won't be picked up either. I have a feeling that checkpot doesn't find this because the parenthesis is not on the same line as the string.

Can you confirm this ends up in the pot file please? Given the checkPot step didn't complain about this line, it may not be available for translators.

AppVeyorBot · 2020-04-24T06:28:50Z

PR introduces Flake8 errors
Build (for testing PR)

See test results for failed build of commit 895246985a

codeofdusk · 2020-04-24T19:58:02Z

It seems that the string is picked up, but not the translator comment:

#. Translators: The label for a setting in general settings to
#. allow NVDA to come up in Windows login screen (useful if user
#. needs to enter passwords or if multiple user accounts are present
#. to allow user to choose the correct account).
#: gui\settingsDialogs.py:727
msgid "Start NVDA during sign-in (requires administrator privileges)"
msgstr ""

#: gui\settingsDialogs.py:741
msgid ""
"Use currently saved settings during sign-in and on secure screens (requires "
"administrator privileges)"
msgstr ""

Moving the parenthesis onto the same line as the string gives me a lint error.

Adriani90 · 2020-04-24T22:32:39Z

@josephsl in case this could have to do with WX python but I am not really sure..

feerrenrut · 2020-04-28T13:54:09Z

Moving the parenthesis onto the same line as the string gives me a lint error.

Which lint error?

Assuming it is "line too long", the following should resolve it, except you then get an error ET113 (flake8-tabs) option continuation-style=hanging does not allow use of alignment as indentation

label=(
	# Translators: blah
	_("Use currently saved settings on the logon and other secure screens"
	" (requires administrator privileges)")
)

For now just disable the "line too long" error. EG:

label=(
	# Translators: blah
	_("Use currently saved settings on the logon and other secure screens (requires administrator privileges)")  # noqa: E501 line too long

Really the translation system should handle one of the following, could you raise an issue please?

# Translators: blah
label=_(
		"Use currently saved settings on the logon and other secure screens"
		" (requires administrator privileges)"
	)

label=_( # Translators: blah
		"Use currently saved settings on the logon and other secure screens"
		" (requires administrator privileges)"
	)

Fix-up of PR nvaccess#10941 Alternative to review action nvaccess#10941 (comment)

) Fix-up of PR #10941 Finding the appropriate code style for a lengthy translatable string along with its required translators comment might not be obvious, even for some of our most seasoned contributors. This problem has led to discussion about various attempts in PR #10941 and the raise of issue #11071. PR #10941 finally resigned by disabling line length checks on a line of the patch. However, as pointed out in a comment on #11071, a styling that complies both with line length and translators comment could finally be found.

PR #10941 brought to light flaws in checkPot regarding handling of long messages - fixed by PR #11093 In #11093 (comment) the creation of unit tests was discussed to help limit occurrence of future regressions. Discussion: #11093 (comment)

Adjust wording of sign-in text

56e98ce

Style.

e806858

Fixes.

af88e2a

feerrenrut suggested changes Apr 6, 2020

View reviewed changes

feerrenrut added the bug label Apr 8, 2020

Review actions.

9b2dc03

More fixes.

824ab8f

feerrenrut suggested changes Apr 22, 2020

View reviewed changes

Fix.

67c8d07

Split long comment.

5913928

Review action.

3b0cd25

codeofdusk mentioned this pull request Apr 30, 2020

Allow splitting of translatable strings #11071

Closed

feerrenrut approved these changes Apr 30, 2020

View reviewed changes

feerrenrut merged commit 5ee03c7 into nvaccess:master Apr 30, 2020

nvaccessAuto added this to the 2020.1 milestone Apr 30, 2020

JulienCochuyt added a commit to accessolutions/nvda that referenced this pull request May 2, 2020

Fix style of translatable string split across multiple lines.

81bd3e0

Fix-up of PR nvaccess#10941 Alternative to review action nvaccess#10941 (comment)

This was referenced May 2, 2020

Fix line too long translation string and comment #11092

Merged

Fix missing translator comment warning for split translatable strings #11093

Merged

JulienCochuyt mentioned this pull request May 19, 2020

checkPot: Add unit tests #11179

Merged

feerrenrut mentioned this pull request Jun 19, 2020

Change wording of nvda at sign in #11281

Merged

	label=_("Start NVDA before Windows sign-in for &everyone (requires administrator privileges)")
	label=_("Use NVDA at Windows sign-in (requires administrator privileges)")

Uh oh!

Conversation

codeofdusk commented Apr 5, 2020 • 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!

AppVeyorBot commented Apr 5, 2020

Uh oh!

codeofdusk commented Apr 6, 2020

Uh oh!

michaelDCurran commented Apr 6, 2020

Uh oh!

codeofdusk commented Apr 6, 2020

Uh oh!

codeofdusk commented Apr 6, 2020

Uh oh!

codeofdusk commented Apr 6, 2020

Uh oh!

michaelDCurran commented Apr 6, 2020

Uh oh!

codeofdusk commented Apr 6, 2020

Uh oh!

Qchristensen commented Apr 6, 2020

Uh oh!

feerrenrut Apr 6, 2020

Choose a reason for hiding this comment

Uh oh!

feerrenrut Apr 6, 2020

Choose a reason for hiding this comment

Uh oh!

feerrenrut Apr 6, 2020

Choose a reason for hiding this comment

Uh oh!

AppVeyorBot commented Apr 22, 2020

Uh oh!

codeofdusk commented Apr 22, 2020

Uh oh!

AppVeyorBot commented Apr 22, 2020

Uh oh!

Uh oh!

feerrenrut Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

feerrenrut Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

AppVeyorBot commented Apr 24, 2020

Uh oh!

codeofdusk commented Apr 24, 2020

Uh oh!

Adriani90 commented Apr 24, 2020

Uh oh!

feerrenrut commented Apr 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codeofdusk commented Apr 5, 2020 •

edited

Loading