10000 [Upstream] [Qt] Standardize and clean up new UI elements by lyricidal · Pull Request #229 · PRCYCoin/PRCYCoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Upstream] [Qt] Standardize and clean up new UI elements #229

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
Merged
merged 6 commits into from
Jun 6, 2022
Merged

[Upstream] [Qt] Standardize and clean up new UI elements #229

merged 6 commits into from
Jun 6, 2022

Conversation

lyricidal
Copy link

This pulls out some UI refactoring and unrelated additions from #509 so they don't get held back.

  • Standardize the use of QToolButton over QPushButton in instances where the button was being
    used as a utility button (copy/paste/addressbook) buttons interacting with text inputs.
  • Standardize the CSS styling of all QToolButton elements.
  • Remove nefarious (empty) placeholderText properties from .ui files.
  • Update strings in BIP38 dialog to match the functionality there.
  • Change from returning Hex to returning WIF private key for decrypted BIP38 key.
  • Update MultiSig/MultiSend UI's to use QToolButton

See individual commit comments for full details.

Rational for using a different button color for Tool Buttons

Tool buttons are never required to be used. They merely offer a shortcut (copy/paste/delete) or a utility to pull an address from the address book. Their scope is narrowly limited to a single UI element, usually a single text entry element.

In contrast, Push Buttons are actionable in a larger scope such as initiating a transaction, confirming a form, or showing coin control. They are typically required in one form or the other to have a user press them to complete the task at hand.

from PIVX-Project/PIVX#537

Somewhat trivial, but as we will be re-enabling some of these menus, better to be more in-line with upstream.

lyricidal added 6 commits June 6, 2022 13:16
@lyricidal
Qt: Refactor layouts for BIP38/SendCoinsEntry/SVM
480ea9f 
Changed multiple instances of QPushButton to QToolButton and updated the corresponding CSS to unify styling across these similar instances.
@lyricidal
Qt: add a paste button to BIP38's decrypt tab
150c139 
Simple QToolButton to grab the contents of the system clipboard and paste it into the encrypted key text box.
@lyricidal
Qt: remove instances of empty placeholderText properties
bbafd94 
At best, these were just extra lines of code, at worst they lead to compile-time errors with versions of Qt below 5.4.

If placeholder text needs to be set for items, best to do so
programatically in the `.cpp` files instead of statically in the `.ui` files.
@lyricidal
Qt: BIP38 dialog string updates
3903f40 
These strings had been copied from the sign/verify message dialog, and seemed out of place. Updated with new strings that (to me) make more sense.
@lyricidal
Qt: BIP38 - return decrypted privkey in WIF format
6fb78a1 
Previously the decrypted privkey was being returned in Hex format, which isn't really usable. Returning it in WIF format is more consistant with how similar user-facing private key operations work.
@lyricidal
Qt: Make Multisend's paste button a QToolButton
29fffaf 
Similar to other UIs, this makes the address paste button in the
Multisend UI a QToolButton using the universal style.
@lyricidal lyricidal added the Upstream Upstream backports/fixes label Jun 6, 2022
@lyricidal lyricidal merged commit 67128cf into develop Jun 6, 2022
@lyricidal lyricidal deleted the upstream-ui_fixup branch June 6, 2022 19:15
anprdev
anprdev reviewed Jun 13, 2022
View reviewed changes
Copy link
@anprdev anprdev left a comment

Choose a reason for hiding this comment

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

Nice idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anprdev anprdev anprdev left review comments

@Shenkyy Shenkyy Shenkyy approved these changes

@Jamesj66 Jamesj66 Jamesj66 approved these changes

@lopeed lopeed Awaiting requested review from lopeed

@UserR256 UserR256 Awaiting requested review from UserR256

Assignees
No one assigned
Labels
Upstream Upstream backports/fixes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lyricidal @Shenkyy @anprdev @Jamesj66
0