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.

Sign up for GitHub

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

Jump to bottom

Add visual accenting for the 'Create new receiving address' button #39

Merged
merged 2 commits into from
Aug 28, 2020
Merged

Add visual accenting for the 'Create new receiving address' button #39

merged 2 commits into from
Aug 28, 2020

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jul 21, 2020 •
edited
Loading

Fix #24

The first commit:

  • visual improvement with no behavior change

The second commit:

  • removes a bunch of LOCs
  • slightly change behavior and makes it standard

With this PR:
DeepinScreenshot_select-area_20200721213040

hebasto added 2 commits July 21, 2020 21:24
@hebasto
qt: Make "Create new receiving address" default unconditionally
4227a8e 
This change makes this button visually accented, and gives to users a
hint about the primary action.
This commit does not change behavior.
@hebasto
qt: Leverage the default "Create new receiving address" button
4ec49f8 
When Enter or Return is pressed the default button will be always
clicked. All buttons can always be clicked from the keyboard by pressing
spacebar when the button has focus.
@hebasto hebasto mentioned this pull request Jul 21, 2020
Closed
@Saibato
Copy link
Contributor

Saibato commented Jul 23, 2020 •
edited
Loading

Concept tACK 4227a8e 4ec49f8

While testing this, at first my intuition was to click the bottom down menu, but there we have no create address.
When i locked closer, i wondered if we could change the bottom down text for show addresses.

from From Receiving addresses to Show all receiving addresses ?
To indicate that we do not create there new addresses

new_receive

@promag
Copy link
Contributor

promag commented Jul 27, 2020

Tested ACK 4ec49f8 on macos.

luke-jr
luke-jr approved these changes Jul 31, 2020
View reviewed changes
Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

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

utACK, minor nit

src/qt/forms/receivecoinsdialog.ui
Comment on lines +117 to +119
<property name="autoDefault">
<bool>false</bool>
</property>
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to mess with this?

Copy link
Member Author

Choose a reason for hiding this comment

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

This change is required.

From Qt docs:

autoDefault : bool
...
This property's default is true for buttons that have a QDialog parent; otherwise it defaults to false.

As ReceiveCoinsDialog is derived from QDialog its buttons have autoDefault == true by default.

Having autoDefault == true breaks rendering of the default button when tabbing through it (when it is focused out). At least on Linux Mint 20 with system Qt 5.12.8.

Copy link
Member

Choose a reason for hiding this comment

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

Weird, sounds like a Qt bug?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe. Didn't track it down though.

@Sjors
Copy link
Member

Sjors commented Aug 28, 2020

utACK

@maflcko maflcko merged commit ca30d34 into bitcoin-core:master Aug 28, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 28, 2020
Merge bitcoin-core/gui#39: Add visual accenting for the 'Create new r…
10e88dd 
…eceiving address' button

4ec49f8 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov)
4227a8e qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov)

Pull request description:

  Fix #24

  The first commit:
  - visual improvement with no behavior change

  The second commit:
  - removes a bunch of LOCs
  - slightly change behavior and makes it standard

  With this PR:
  ![DeepinScreenshot_select-area_20200721213040](https://user-images.githubusercontent.com/32963518/88093294-7b2a6700-cb9a-11ea-89a2-a0e2678056a7.png)

ACKs for top commit:
  Saibato:
    Concept tACK  bitcoin-core/gui@4227a8e bitcoin-core/gui@4ec49f8
  promag:
    Tested ACK 4ec49f8 on macos.

Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
@hebasto hebasto deleted the 200721-prim branch August 28, 2020 17:44
apoelstra added a commit to apoelstra/elements that referenced this pull request Dec 3, 2020
@apoelstra
Merge ca30d34 into merged_master (Bitcoin PR bitcoin-core/gui#39)
9ddbb5e
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Mar 22, 2021
@apoelstra
Merge ca30d34 into merged_master (Bitcoin PR bitcoin-core/gui#39)
2bb4241
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 27, 2021
@PastaPastaPasta
Merge bitcoin-core/gui#39: Add visual accenting for the 'Create new r…
b3179a8 
…eceiving address' button

4ec49f8 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov)
4227a8e qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov)

Pull request description:

  Fix #24

  The first commit:
  - visual improvement with no behavior change

  The second commit:
  - removes a bunch of LOCs
  - slightly change behavior and makes it standard

  With this PR:
  ![DeepinScreenshot_select-area_20200721213040](https://user-images.githubusercontent.com/32963518/88093294-7b2a6700-cb9a-11ea-89a2-a0e2678056a7.png)

ACKs for top commit:
  Saibato:
    Concept tACK  bitcoin-core/gui@4227a8e bitcoin-core/gui@4ec49f8
  promag:
    Tested ACK 4ec49f8 on macos.

Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 28, 2021
@PastaPastaPasta
Merge bitcoin-core/gui#39: Add visual accenting for the 'Create new r…
22c4ede 
…eceiving address' button

4ec49f8 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov)
4227a8e qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov)

Pull request description:

  Fix #24

  The first commit:
  - visual improvement with no behavior change

  The second commit:
  - removes a bunch of LOCs
  - slightly change behavior and makes it standard

  With this PR:
  ![DeepinScreenshot_select-area_20200721213040](https://user-images.githubusercontent.com/32963518/88093294-7b2a6700-cb9a-11ea-89a2-a0e2678056a7.png)

ACKs for top commit:
  Saibato:
    Concept tACK  bitcoin-core/gui@4227a8e bitcoin-core/gui@4ec49f8
  promag:
    Tested ACK 4ec49f8 on macos.

Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 29, 2021
@PastaPastaPasta
Merge bitcoin-core/gui#39: Add visual accenting for the 'Create new r…
d345a79 
…eceiving address' button

4ec49f8 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov)
4227a8e qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov)

Pull request description:

  Fix #24

  The first commit:
  - visual improvement with no behavior change

  The second commit:
  - removes a bunch of LOCs
  - slightly change behavior and makes it standard

  With this PR:
  ![DeepinScreenshot_select-area_20200721213040](https://user-images.githubusercontent.com/32963518/88093294-7b2a6700-cb9a-11ea-89a2-a0e2678056a7.png)

ACKs for top commit:
  Saibato:
    Concept tACK  bitcoin-core/gui@4227a8e bitcoin-core/gui@4ec49f8
  promag:
    Tested ACK 4ec49f8 on macos.

Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 1, 2021
@PastaPastaPasta
Merge bitcoin-core/gui#39: Add visual accenting for the 'Create new r…
6bf967c 
…eceiving address' button

4ec49f8 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov)
4227a8e qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov)

Pull request description:

  Fix #24

  The first commit:
  - visual improvement with no behavior change

  The second commit:
  - removes a bunch of LOCs
  - slightly change behavior and makes it standard

  With this PR:
  ![DeepinScreenshot_select-area_20200721213040](https://user-images.githubusercontent.com/32963518/88093294-7b2a6700-cb9a-11ea-89a2-a0e2678056a7.png)

ACKs for top commit:
  Saibato:
    Concept tACK  bitcoin-core/gui@4227a8e bitcoin-core/gui@4ec49f8
  promag:
    Tested ACK 4ec49f8 on macos.

Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 1, 2021
@PastaPastaPasta
Merge bitcoin-core/gui#39: Add visual accenting for the 'Create new r…
fe2992e 
…eceiving address' button

4ec49f8 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov)
4227a8e qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov)

Pull request description:

  Fix #24

  The first commit:
  - visual improvement with no behavior change

  The second commit:
  - removes a bunch of LOCs
  - slightly change behavior and makes it standard

  With this PR:
  ![DeepinScreenshot_select-area_20200721213040](https://user-images.githubusercontent.com/32963518/88093294-7b2a6700-cb9a-11ea-89a2-a0e2678056a7.png)

ACKs for top commit:
  Saibato:
    Concept tACK  bitcoin-core/gui@4227a8e bitcoin-core/gui@4ec49f8
  promag:
    Tested ACK 4ec49f8 on macos.

Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 15, 2021
@PastaPastaPasta
Merge bitcoin-core/gui#39: Add visual accenting for the 'Create new r…
7abcce9 
…eceiving address' button

4ec49f8 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov)
4227a8e qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov)

Pull request description:

  Fix #24

  The first commit:
  - visual improvement with no behavior change

  The second commit:
  - removes a bunch of LOCs
  - slightly change behavior and makes it standard

  With this PR:
  ![DeepinScreenshot_select-area_20200721213040](https://user-images.githubusercontent.com/32963518/88093294-7b2a6700-cb9a-11ea-89a2-a0e2678056a7.png)

ACKs for top commit:
  Saibato:
    Concept tACK  bitcoin-core/gui@4227a8e bitcoin-core/gui@4ec49f8
  promag:
    Tested ACK 4ec49f8 on macos.

Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 16, 2021
@PastaPastaPasta
Merge bitcoin-core/gui#39: Add visual accenting for the 'Create new r…
a560d32 
…eceiving address' button

4ec49f8 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov)
4227a8e qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov)

Pull request description:

  Fix #24

  The first commit:
  - visual improvement with no behavior change

  The second commit:
  - removes a bunch of LOCs
  - slightly change behavior and makes it standard

  With this PR:
  ![DeepinScreenshot_select-area_20200721213040](https://user-images.githubusercontent.com/32963518/88093294-7b2a6700-cb9a-11ea-89a2-a0e2678056a7.png)

ACKs for top commit:
  Saibato:
    Concept tACK  bitcoin-core/gui@4227a8e bitcoin-core/gui@4ec49f8
  promag:
    Tested ACK 4ec49f8 on macos.

Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Sep 17, 2021
@hebasto @PiRK
Add visual accenting for the 'Create new receiving address' button
483cb49 
Summary:
> qt: Make "Create new receiving address" default unconditionally
>
>  This change makes this button visually accented, and gives to users a
> hint about the primary action.
> This commit does not change behavior.

> qt: Leverage the default "Create new receiving address" button
>
> When Enter or Return is pressed the default button will be always
> clicked. All buttons can always be clicked from the keyboard by pressing
> spacebar when the button has focus.

This is basically a simplification of the way pressing return in the
"Receive" tab of bitcoin-qt submits the form, doing it the standard Qt
way without additional code.

This is a backport of [[bitcoin-core/gui#39 | core-gui#39]]

Test Plan:
`ninja && src/qt/bitcoin-qt`

In the Receive tab of the main window, verify that pressing Return still
submits the form and creates a new address.

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10135
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@luke-jr luke-jr luke-jr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Give primary action items more visual weight
6 participants
@hebasto @Saibato @promag @Sjors @luke-jr @maflcko