Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[qt] Fix typo and access key in optionsdialog.ui #10911

Merged
merged 1 commit into from Sep 7, 2017

Conversation

Projects
None yet
9 participants
@keystrike
Copy link
Contributor

keystrike commented Jul 22, 2017

Tooltip displayed ampersand incorrectly, & should be in text property rather than tooltip so that access key is correctly displayed for accessibility.

@fanquake fanquake added the GUI label Jul 22, 2017

@benma

This comment has been minimized.

Copy link
Contributor

benma commented Jul 24, 2017

utACK f066abd0837df5f4d414d6a092e48f334f9bb864

@jonasschnelli

This comment has been minimized.

Copy link
Member

jonasschnelli commented Jul 24, 2017

I think the PR description is a little bit misleading because it adds new access keys, right?
Are there no overlapping access keys (haven't checked)?

Binaries: https://bitcoin.jonasschnelli.ch/build/229

@benma

This comment has been minimized.

Copy link
Contributor

benma commented Jul 24, 2017

As part of my utACK I checked that there is no overlap. Even if there was, it would just cycle through (there is already an instance of that with the two Proxy IP in the network tab).

@jonasschnelli

This comment has been minimized.

Copy link
Member

jonasschnelli commented Jul 24, 2017

utACK f066abd0837df5f4d414d6a092e48f334f9bb864
Please update PR description (it's now part of the git history).

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Jul 24, 2017

utACK

@laanwj laanwj changed the title Typo in optionsdialog.ui [qt] Fix typo and access key in optionsdialog.ui Jul 24, 2017

@promag

This comment has been minimized.

Copy link
Member

promag commented Jul 24, 2017

utACK f066abd. Maybe squash?

@@ -199,10 +199,10 @@
<item>
<widget class="QCheckBox" name="allowIncoming">
<property name="toolTip">
<string>Accept connections from outside</string>
<string>Accept connections from outside.</string>

This comment has been minimized.

@TheBlueMatt

TheBlueMatt Jul 25, 2017

Contributor

Will this result in a translation string change?

This comment has been minimized.

@luke-jr

luke-jr Jul 26, 2017

Member

Yes, but one that is easy to adjust for.

@TheBlueMatt

This comment has been minimized.

Copy link
Contributor

TheBlueMatt commented Jul 25, 2017

I assume &amps will not result in translation string changes?

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Jul 26, 2017

I assume &amps will not result in translation string changes?

They do.
This should go in after 0.15 branch.

@luke-jr
Copy link
Member

luke-jr left a comment

Since this touches strings, I think we should merge the fixes, but leave the hotkeys alone/unchanged for now.

Poke me to save affected translations just before merging, so I can restore them after.

</property>
<property name="text">
<string>Allow incoming connections</string>
<string>Allow incomin&amp;g connections</string>

This comment has been minimized.

@luke-jr

luke-jr Jul 26, 2017

Member

This is currently being auto-assigned L. I'm not sure G is logical enough to warrant a change.

This comment has been minimized.

@luke-jr

luke-jr Jul 26, 2017

Member

Suggest leaving this alone until 0.16.

This comment has been minimized.

@keystrike

keystrike Jul 26, 2017

Author Contributor

I don't see any of these being auto-assigned under Windows or Linux. Perhaps I'm missing something?

@@ -399,7 +399,7 @@
<string>Connect to the Bitcoin network through a separate SOCKS5 proxy for Tor hidden services.</string>
</property>
<property name="text">
<string>Use separate SOCKS5 proxy to reach peers via Tor hidden services:</string>
<string>Use separate SOCKS&amp;5 proxy to reach peers via Tor hidden services:</string>

This comment has been minimized.

@luke-jr

luke-jr Jul 26, 2017

Member

This is being auto-assigned S. If we change it, T seems like a more logical choice...

This comment has been minimized.

@luke-jr

luke-jr Jul 26, 2017

Member

Suggest leaving it alone until 0.16.

</property>
<property name="text">
<string>Hide tray icon</string>
<string>&amp;Hide tray icon</string>

This comment has been minimized.

@luke-jr

luke-jr Jul 26, 2017

Member

Redundant with auto-assignment. Suggest leaving it alone until 0.16. (But still fix the tooltip)

@@ -610,7 +610,7 @@
<string>Third party URLs (e.g. a block explorer) that appear in the transactions tab as context menu items. %s in the URL is replaced by transaction hash. Multiple URLs are separated by vertical bar |.</string>
</property>
<property name="text">
<string>Third party transaction URLs</string>
<string>&amp;Third party transaction URLs</string>

This comment has been minimized.

@luke-jr

luke-jr Jul 26, 2017

Member

Redundant with auto-assignment. Suggest leaving it alone until 0.16.

@jonasschnelli

This comment has been minimized.

Copy link
Member

jonasschnelli commented Aug 15, 2017

@keystrike: can you squash these two commits?

@fanquake fanquake referenced this pull request Sep 2, 2017

Closed

Update optionsdialog.ui #11206

@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Sep 7, 2017

Typo in optionsdialog.ui
Tooltip displayed ampersand incorrectly, &amp; should be in text.

@jonasschnelli jonasschnelli force-pushed the keystrike:master branch to d2be7b2 Sep 7, 2017

@jonasschnelli jonasschnelli merged commit d2be7b2 into bitcoin:master Sep 7, 2017

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

jonasschnelli added a commit that referenced this pull request Sep 7, 2017

Merge #10911: [qt] Fix typo and access key in optionsdialog.ui
d2be7b2 Typo in optionsdialog.ui Tooltip displayed ampersand incorrectly, &amp; should be in text. (James Evans)

Pull request description:

  Tooltip displayed ampersand incorrectly, &amp; should be in text property rather than tooltip so that access key is correctly displayed for accessibility.

Tree-SHA512: 331848207317d37d4d9db40119d0b7ae9a276d06cd1b057cd0e87d508e1aa769b785246ca30ca9156db632798ec9f68ba8bf78cf42904267b4187bd27cfced35
@jonasschnelli

This comment has been minimized.

Copy link
Member

jonasschnelli commented Sep 7, 2017

squashed by maintainer due to lack of response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.