Fix Shortcut Ambiguities, Clean up text

jarolrod commented at 10:23 pm on November 5, 2020: member

Removes shortcut ambiguities found in the Settings -> Options menu. Builds upon this rationale.

Before/Master

master

After/PR

Main Tab

Start Bitcoin Core on system login
- Moved shortcut from [S]tart to -> [B]itcoin in order to reserve S for Size of database cache
Prune block storage to
- Moved shortcut from [B]lock to -> [P]rune in order to reserve B for Start Bitcoin Core on system login
- Added a colon at the end since this text denotes a data field
Size of database cache
- Moved shortcut from [D]atabase to -> [S]ize in order to fix the conflict with the Display tab shortcut
- Added a colon at the end since this text denotes a data field
Number of script verfication threads
- Added a colon at the end since this text denotes a data field

Wallet Tab

Enable coin control features
- Moved shortcut from [C]oin to -> [E]nable in order to fix conflict with the Cancel button

Network Tab

`Connect through SOCKS5 proxy (default proxy):
- Moved shortcut from [C]onnect to -> SOCKS[5] in order to fix conlict with the Cancel button

Window Tab

Minimize to the tray instead of the taskbar
- Moved shortcut from [M]inimize to -> [T]ray in order to fix conflict with the Main tab

Closes #126

qt: Fix Shortcut Ambiguities, Clean up text 401a5e70de

jarolrod renamed this:
~~qt: Fix Shortcut Ambiguities, Clean up text~~
GUI: Fix Shortcut Ambiguities, Clean up text
on Nov 5, 2020

hebasto commented at 11:14 am on November 6, 2020: member

Concept ACK.

The “GUI:” prefix is redundant in the GUI repo PR title (but the “qt: " prefix in the commit message is correct).

Generally, screenshots “before”/“master” and “after”/“pr” are expected as a part of the OP in the GUI repo :)

hebasto commented at 12:46 pm on November 6, 2020: member

Approach ACK 401a5e70de42e6ddf0b5bc79d96ea8d670851860.

About adding : character:

“Third party transaction URLs” string could be ended with :
“Prune block storage to <NN> GB” seems a complete simple sentence, and adding an : is not required, but I’m not an expert in the field of English punctuation rules :)

hebasto commented at 12:49 pm on November 6, 2020: member

ping @Bosch-0 @GBKS @johnsBeharry

jarolrod renamed this:
~~GUI: Fix Shortcut Ambiguities, Clean up text~~
qt: Fix Shortcut Ambiguities, Clean up text
on Nov 6, 2020

Bosch-0 commented at 5:17 am on November 9, 2020: none

Testing ACK 7e373294a5ae819099c39d9d03d1f5a311d63cfc on Windows Windows 10.0.18363 Build 18363 and Ubuntu 20.04.

Looks good to me!

jarolrod commented at 10:14 pm on November 10, 2020: member

Approach ACK 401a5e7.

About adding : character:

* "Third party transaction URLs" string could be ended with `:`

* "Prune block storage to <NN> GB" seems a complete simple sentence, and adding an `:` is not required, but I'm not an expert in the field of English punctuation rules :)

While that is a complete sentence, I think the “Prune block storage to: [NN] GB” makes this field visually and functionally consistent with the other data fields it is surrounded by.

hebasto commented at 7:23 pm on November 11, 2020: member

About adding : character:

“Third party transaction URLs” string could be ended with :

?

hebasto commented at 7:25 pm on November 11, 2020: member

@Bosch-0

Testing ACK 7e37329 on Windows Windows 10.0.18363 Build 18363 and Ubuntu 20.04.

Looks good to me!

Currently, the top commit of this PR is 401a5e70de42e6ddf0b5bc79d96ea8d670851860 rather 7e37329 :)

add colon to third party URLS data field 88ecffa9d3

jarolrod commented at 4:40 am on November 12, 2020: member

Added a colon to the Third party transaction URLs feature under the Display Tab as suggested by @hebasto.

Before/Master

master-third-party-txs

After/PR 88ecffa

pr-third-party-txs

in src/qt/forms/optionsdialog.ui:36 in 88ecffa9d3

32@@ -33,7 +33,7 @@
33           <string>Automatically start %1 after logging in to the system.</string>
34          </property>
35          <property name="text">
36-          <string>&amp;Start %1 on system login</string>

luke-jr commented at 7:42 pm on November 13, 2020:

Not sure it’s a good idea to tie the shortcut to PACKAGE_NAME… This option is more about startup anyway: [S] is a good fit.

in src/qt/forms/optionsdialog.ui:61 in 88ecffa9d3

57@@ -58,7 +58,7 @@
58            <string>Disables some advanced features but all blocks will still be fully validated. Reverting this setting requires re-downloading the entire blockchain. Actual disk usage may be somewhat higher.</string>
59           </property>
60           <property name="text">
61-           <string>Prune &amp;block storage to</string>
62+           <string>&amp;Prune block storage to:</string>

luke-jr commented at 7:42 pm on November 13, 2020:

ACK despite no need

in src/qt/forms/optionsdialog.ui:108 in 88ecffa9d3

104@@ -105,7 +105,7 @@
105          <item>
106           <widget class="QLabel" name="databaseCacheLabel">
107            <property name="text">
108-            <string>Size of &amp;database cache</string>
109+            <string>&amp;Size of database cache:</string>

luke-jr commented at 7:42 pm on November 13, 2020:

Don’t see a good option here. Maybe just fall back to [I]?

in src/qt/forms/optionsdialog.ui:215 in 88ecffa9d3

211@@ -212,7 +212,7 @@
212              <string>Whether to show coin control features or not.</string>
213             </property>
214             <property name="text">
215-             <string>Enable coin &amp;control features</string>
216+             <string>&amp;Enable coin control features</string>

luke-jr commented at 7:43 pm on November 13, 2020:

Meh, okay.

in src/qt/forms/optionsdialog.ui:278 in 88ecffa9d3

274@@ -275,7 +275,7 @@
275           <string>Connect to the Bitcoin network through a SOCKS5 proxy.</string>
276          </property>
277          <property name="text">
278-          <string>&amp;Connect through SOCKS5 proxy (default proxy):</string>
279+          <string>Connect through SOCKS&amp;5 proxy (default proxy):</string>

luke-jr commented at 7:43 pm on November 13, 2020:

ACK

in src/qt/forms/optionsdialog.ui:586 in 88ecffa9d3

582@@ -583,7 +583,7 @@
583           <string>Show only a tray icon after minimizing the window.</string>
584          </property>
585          <property name="text">
586-          <string>&amp;Minimize to the tray instead of the taskbar</string>
587+          <string>Minimize to the &amp;tray instead of the taskbar</string>

luke-jr commented at 7:43 pm on November 13, 2020:

ACK

luke-jr changes_requested

hebasto commented at 7:31 am on November 15, 2020: member

https://www.nngroup.com/articles/ui-copy/ (the “Guidelines for Command Shortcuts” part) could be relevant.

promag commented at 11:23 pm on November 15, 2020: contributor

Concept ACK.

jonasschnelli commented at 10:06 am on December 1, 2020: contributor

Have we though how this would behave after translations? I guess the translators would define the shortcuts for non English languages?

jarolrod marked this as a draft on Feb 19, 2021

hebasto commented at 4:14 am on March 15, 2021: member

Here are some excerpts from Qt docs:

https://doc.qt.io/qt-5/linguist-overview.html

Keyboard accelerators may need to be changed but without introducing conflicts… We cannot use a letter that is already in use - unless we change several accelerators.

https://doc.qt.io/qt-5/linguist-translators.html#changing-keyboard-accelerators

Conflicts with other Alt key accelerators must be avoided within a context. Note that some Alt key accelerators, usually those on the menu bar, may apply in other contexts. @jonasschnelli Have we though how this would behave after translations? I guess the translators would define the shortcuts for non English languages?

With the current translation process it is a translators’ responsibility to (re)define shortcuts, right?

hebasto renamed this:
~~qt: Fix Shortcut Ambiguities, Clean up text~~
Fix Shortcut Ambiguities, Clean up text
on Aug 5, 2021

DrahtBot commented at 11:14 pm on December 13, 2021: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

DrahtBot added the label Needs rebase on Dec 13, 2021

DrahtBot commented at 12:11 pm on March 21, 2022: contributor

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

jarolrod commented at 11:55 pm on April 3, 2022: member

leaving up for grabs

jarolrod closed this on Apr 3, 2022

jarolrod added the label Up for grabs on Apr 3, 2022

jarolrod removed the label Needs rebase on Jul 25, 2022

jarolrod removed the label Up for grabs on Jul 25, 2022

bitcoin-core locked this on Jul 25, 2023

Fix Shortcut Ambiguities, Clean up text #129

Before/Master

After/PR

Main Tab

Wallet Tab

Network Tab

Window Tab

Before/Master

After/PR 88ecffa