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

gui: send amount placeholder value #17195

Merged
merged 1 commit into from Oct 21, 2019

Conversation

@JeremyCrookshank
Copy link
Contributor

JeremyCrookshank commented Oct 18, 2019

Noticed that there wasn't a default value for the send amount. However if you put a value in or click the up and down arrows you're unable to get it blank again, so it makes sense that it has a default value. I hope this also makes it more clear that users can send less than 1 BTC if it shows the 8 decimal places

PR:
Capture

@emilengler

This comment has been minimized.

Copy link
Contributor

emilengler commented Oct 18, 2019

Concept ACK

@fanquake fanquake added the GUI label Oct 18, 2019
@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Oct 19, 2019

NACK. I prefer the blank field, as it allows for pasting amounts (which is very common when doing currency conversions and such, or to paste the exact amount from orders).

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Oct 19, 2019

Maybe use setPlaceholderText? (though be careful: the number of decimals would depend on the configured unit)

@kristapsk

This comment has been minimized.

Copy link
Contributor

kristapsk commented Oct 19, 2019

I agree with @laanwj here.

@JeremyCrookshank JeremyCrookshank force-pushed the JeremyCrookshank:defaultsendamount branch from a2e3f8b to b6e34af Oct 19, 2019
@JeremyCrookshank

This comment has been minimized.

Copy link
Contributor Author

JeremyCrookshank commented Oct 19, 2019

I've attempted that now. Hope it's okay!

PR:
1
2
3
4

@JeremyCrookshank JeremyCrookshank changed the title gui: a default send amount value gui: send amount placeholder value Oct 19, 2019
@promag

This comment has been minimized.

Copy link
Member

promag commented Oct 19, 2019

ACK 57e2ede.

@Danny-Scott

This comment has been minimized.

Copy link
Contributor

Danny-Scott commented Oct 19, 2019

ACK - placeholder text makes complete sense here.

@fanquake fanquake requested a review from laanwj Oct 19, 2019
@emilengler

This comment has been minimized.

Copy link
Contributor

emilengler commented Oct 19, 2019

@laanwj You just need to do one click more to select the whole text and then you can just replace it (at least on most systems).

@GChuf

This comment has been minimized.

Copy link
Contributor

GChuf commented Oct 20, 2019

ACK 57e2ede

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Oct 21, 2019

ACK 57e2ede, this is a surprisingly compact solution too

@laanwj You just need to do one click more to select the whole text and then you can just replace it (at least on most systems).

quick, patent Only One More Click ™️ payments

laanwj added a commit that referenced this pull request Oct 21, 2019
57e2ede Send amount shows minimum amount placeholder (JeremyCrookshank)

Pull request description:

  Noticed that there wasn't a default value for the send amount. However if you put a value in or click the up and down arrows you're unable to get it blank again, so it makes sense that it has a default value. I hope this also makes it more clear that users can send less than 1 BTC if it shows the 8 decimal places

  PR:
  ![Capture](https://user-images.githubusercontent.com/46864828/67132088-549c6180-f1ff-11e9-9ba5-67fdcd6db894.PNG)

ACKs for top commit:
  promag:
    ACK 57e2ede.
  GChuf:
    ACK 57e2ede
  laanwj:
    ACK 57e2ede, this is a surprisingly compact solution too

Tree-SHA512: 354590d2a88231b8649f7ae985c8a7864d74ca0e1f8603cb1730ba46747084de90ee6285ce4d39ee04b054fb9cd2d78ebc71146f3af694c37a8a3aff7f051800
@laanwj laanwj merged commit 57e2ede into bitcoin:master Oct 21, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Nov 4, 2019

@laanwj You just need to do one click more to select the whole text and then you can just replace it (at least on most systems).

Pasting an amount to an empty field is a single middle click, nothing more.

Replacing text would be a double-click (maybe triple on some systems?), then Ctrl-V on the keyboard. Significantly more complicated.

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Nov 15, 2019
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Nov 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants
You can’t perform that action at this time.