Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Refactor buttons on about:preferences#payments and bookmark dialog #5705

Merged
merged 2 commits into from
Nov 23, 2016
Merged

Refactor buttons on about:preferences#payments and bookmark dialog #5705

merged 2 commits into from
Nov 23, 2016

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Nov 17, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Fixes #4786
Fixed #5894

  • Moved paddings and width in "whiteButton" to "wideButton"
  • Added "wideButton" to the advanced settings button on about:preferences#payments
  • Added font-size to the button
  • Added display:flex to footer of advanced settings
  • Added white-space:nowrap to "buttonCommon" (buttons will not be wrapped by default)
  • Removed redundant classes from bookmarkButtons to fix the size difference between "Remove" and "Done" buttons, which was introduced with the commit 85e428a
  • Introduced new classes "removeButton" and "doneButton" inside bookmarkButtons to make the width of the row same as the other rows
  • Introduced a new class "copyButton" to keep the font-size of the recovery key copy buttons to 14px
  • Indroduced a flexbox to copyKeyContainer to keep the buttons aligned

Auditors: @jkup @bradleyrichter

Test Plan: explained in messages of the commits 75a754a 23207ed

@luixxiul luixxiul added refactoring feature/rewards design A design change, especially one which needs input from the design team. labels Nov 17, 2016
@luixxiul
Copy link
Contributor Author

luixxiul commented Nov 19, 2016

With 23207ed

screenshot 2016-11-20 1 52 29

Edit: please make sure the button styling was changed with #6000

@luixxiul luixxiul changed the title Refactor buttons on about:preferences#payments Refactor buttons on about:preferences#payments and Bookmark dialog on the URL bar Nov 19, 2016
@luixxiul luixxiul changed the title Refactor buttons on about:preferences#payments and Bookmark dialog on the URL bar Refactor buttons on about:preferences#payments and bookmark dialog on the URL bar Nov 19, 2016
@luixxiul
Copy link
Contributor Author

luixxiul commented Nov 20, 2016

With 23207ed the bookmark dialogs becomes like these:

screenshot 2016-11-20 10 46 26

screenshot 2016-11-20 11 46 34

The difference of font-size between "Remove" and "Done" which exists on master was fixed on both of the dialogs

Edit:
With 1df8dc3 of #6000 (comment), the flex-grow property was removed to make the buttons like these:

screenshot 2016-12-03 17 24 45

Issue #6004

The remaining task is: #6400

@luixxiul luixxiul changed the title Refactor buttons on about:preferences#payments and bookmark dialog on the URL bar Refactor buttons on about:preferences#payments and bookmark dialog Nov 20, 2016
@bradleyrichter
Copy link
Contributor

@luixxiul This is awesome. Thanks for the great cleanup work here.

Suguru Hirahara added 2 commits November 22, 2016 10:06
Fixes #4786

- Moved paddings and width in "whiteButton" to "wideButton"
- Added "wideButton" to the advanced settings button on about:preferences#payments
- Added font-size to the button
- Added display:flex to footer of advanced settings
- Added white-space:nowrap to "buttonCommon" (buttons will not be wrapped by default)

Auditors: @jkup

Test Plan:

1. Open about:preferences#payments
2. Make sure the font size of the two buttons "Add funds" and "Advanced Settings..." is equal
3. Click "Advanced Settings..."
4. Make sure the buttons on the footer are aligned center
5. Make sure each button is not wrapped
- Removed redundant classes from bookmarkButtons to fix the size difference between "Remove" and "Done" buttons, which was introduced with the commit 85e428a
- Introduced new classes "removeButton" and "doneButton" inside bookmarkButtons to make the width of the row same as the other rows
- Introduced a new class "copyButton" to keep the font-size of the recovery key copy buttons to 14px
- Indroduced a flexbox to copyKeyContainer to keep the buttons aligned

Auditors: @bradleyrichter

Test Plan:
1. Click the bookmark button on the URL bar
2. Make sure height of "Remove" and "Done" buttons is equal
3. Make sure "Remove" button is longer than "Done" button
4. Make sure color of "Remove" label is no longer white on hover
5. Make sure width of the row of the buttons is equal to the other rows
6. Open about:bookmarks and edit a bookmark
7. Make sure height and width of "Remove" and "Done" are equal
8. Open about:preferences#payments, click Advanced Settings and Backup your wallet
9. Make sure keys and buttons are aligned
10. Make sure font-size of the labels "Copy" is 14px
@luixxiul
Copy link
Contributor Author

Rebased and ready for a review again @bradleyrichter

@bsclifton
Copy link
Member

bsclifton commented Nov 22, 2016

This looks great! 😄

@jkup do these changes affect what you're working on? I know you're in the process of converting buttons over to an Aphrodite component

@bsclifton bsclifton added this to the 0.13.0 milestone Nov 22, 2016
@jkup
Copy link
Contributor

jkup commented Nov 22, 2016

@bsclifton nope this is all good! Been keeping an eye on these changes!

@bsclifton
Copy link
Member

bsclifton commented Nov 22, 2016

@luixxiul killer job with all your recent PRs; the style updates definitely help Brave feel more polished. Keep up the great work! 😄

@luixxiul
Copy link
Contributor Author

OK, I think it's good to merge..

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
design A design change, especially one which needs input from the design team. feature/bookmarks feature/rewards misc/button refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Remove Bookmark" button hard to read on hover l10n of ledger advanced settings modal window
4 participants