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

Normalize modal overlay dialogs #8666

Merged
merged 10 commits into from
May 4, 2017
Merged

Normalize modal overlay dialogs #8666

merged 10 commits into from
May 4, 2017

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented May 3, 2017 •
edited by bsclifton
Loading

TODO: add modal dialog to about:styles.

Normalize modal overlay dialogs.
Follow up to #8042

Closes #6372
Closes #8054
Closes #8662
Closes #8663
Addresses #8661
Addresses #8054
Addresses #8661

Test Plan 1:

  1. npm run test -- --grep='Payments'
  2. npm run test -- --grep='Sync Panel'

Test Plan 2:

  1. Open about:preferences#payments
  2. Click "Add funds"
  3. Make sure the 3 icons inside the dialog body is aligned

Test plan 3:

  1. Open about:preferences#payments
  2. Click "Add funds"
  3. Click "Fund with credit/debit"
  4. Make sure the coinbase widget is displayed at the center

Test Plan 4:

  1. Add your country code to coinbaseCountries.js
  2. Open about:preferences#payments
  3. Click "Add funds"
  4. Click "Display QR code"
  5. Make sure the 3 icons appear on the footer

Test Plan 5:

  1. Open about:preferences#payments
  2. Enable payments
  3. Click "Add funds"
  4. Click "Display QR Code"
  5. Make sure both the QR image and the label is aligned at the center

Test Plan 6:

  1. Enable coinbase widget (via VPN)
  2. Open about:preferences#payments
  3. Click "Add funds"
  4. Make sure "Done" button is aligned with the other buttons
  5. Disable the coinbase widget
  6. Make sure "Done button" is aligned to the right side

Test Plan 7:

  1. Open about:preferences#payments
  2. Click "Add funds"
  3. Click "Display QR code"
  4. Hover on the QR code image
  5. Make sure its title is displayed

Test Plan 8:

  1. Open about:preferences#sync
  2. Enable Sync
  3. Click "Sync a new device"
  4. Make sure just padding exists under "Show secret code words"
  5. Click "Show secret code words"
  6. Make sure the code is selectable

screenshot 2017-05-04 2 24 35

screenshot 2017-05-04 2 24 43

screenshot 2017-05-04 2 27 10

screenshot 2017-05-04 2 27 20

  • 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).

Suguru Hirahara and others added 10 commits May 4, 2017 02:55
Refactor modalOverlay.js with Aphrodite, polish Sync and Payments stuff
2a0d0a7 
1. advancedSettings.js

- Remove commonStyles.modalOverlay__footer and commonStyles.modalOverlay__footerButton
- Remove padding from advancedSettings thanks to modal dialog normalization

2. bitcoinDashboard.js

- Add no-important to Aphrodite
- Replace paymentCommon.panelFooter with styles.dashboardFooter with temp__displayNone
- Replace qrcodeOverlay with modalOverlay__qrcodeOverlay

3. ledgerBackup.js

- Remove commonStyles.modalOverlay__footer and commonStyles.modalOverlay__footerButton

4. ledgerRecovery.js

- Remove commonStyles.modalOverlay__footer and commonStyles.modalOverlay__footerButton

5. paymentsTab.js

- Add paymentHistoryOverlay__header
- Add paymentHistoryOverlay__footer
- Add paymentHistoryOverlay__body
- Add temp__displayNone temporarily

6. syncTab.js

- Rename overlayContent to newOverlayContent
- Add newOverlayFooter
- Remove syncOverlayBody__wrapper
- Remove commonStyles.modalOverlay__footer and commonStyles.modalOverlay__footerButton
- Remove commonStyles.modalOverlay__dialog and commonStyles.modalOverlay__title thanks to modal dialog normalization
- Set user-select:none to syncOverlayBody__wrapper

7. commonStyles.js - Remove modalOverlay__* thanks to modal dialog normalization (Addresses #8054)

8. global.js - Add zindexModal

9. payment.js - Remove panelFooter thanks to modal dialog normalization

10. modalOverlay.js - Refactor with Aphrodite (Closes #8054)

dialog
|- dialog__wrapper_modal
|- dialog__wrapper_last
|- dialog__wrapper_transparentBackground
|-- dialog__header
|--- dialog__header__close
|--- dialog__header__title
|-- dialog__body__wrapper
|--- dialog__body
|-- dialog__footer

11. preferences.less - Remove obsolete code (Addresses #8661)

12. modalOverlay.less - Remove obsolete code (Addresses #8054)
@cezaraugusto
payment modal dialog - bitcoinDashboard.js - move footer to modal
c7ed725
payment modal dialog cleanup 1 - ledgerBackup.js and ledgerRecovery.js
32dad70
payment modal dialog cleanup 2 - advancedSettings.js
989883e
payment modal dialog cleanup 3
245d232 
Remove paymentCommon that become obsolete thanks to modal dialog normalization

Also:
- Cleanup styles
- Add registerMock to paymentsTabTest.js

Test Plan:
1. npm run test -- --grep='Payments'
payment modal dialog - refactor bitcoinDashboard.js
2a7695a 
Normalize Add funds modal dialog based on the refactored modalOverlay.js
- Reorganize functions
- Reorganize styles to make the dialog consistent with other dialogs

Rename classnames on the BEM style

panel
|- panel__divider
|-- panel__divider_left
|--- panel__divider_left__titleWrapper
|---- panel__divider_left__titleWrapper__iconWrapper
|--- panel__divider_left__listTitleWrapper
|---- panel__divider_left__listTitleWrapper__title
|---- panel__divider_left__listTitleWrapper__subTitle
|-- panel__divider_right
|--- panel__divider_right__title
|--- panel__divider_right__subTitle
|--- panel__divider_right__bitcoinPanel__paymentURL
|--- panel__divider_right__bitcoinPanel__walletLabelText
|--- panel__divider_right__bitcoinPanel__walletAddressText

modalOverlay__qrcodeOverlay
|- modalOverlay__qrcodeOverlay__content
|-- modalOverlay__qrcodeOverlay__content__bitcoinQR
|- modalOverlay__qrcodeOverlay__footerWrapper
|-- modalOverlay__qrcodeOverlay__footerWrapper__footer
|--- modalOverlay__qrcodeOverlay__footerWrapper__footer__qrcodeLogo
|--- modalOverlay__qrcodeOverlay__footerWrapper__footer__appstoreLogo
|--- modalOverlay__qrcodeOverlay__footerWrapper__footer__playstoreLogo

modalOverlay__coinbaseOverlay
|- modalOverlay__coinbaseOverlay__header
|- modalOverlay__coinbaseOverlay__bodyWrapper
|-- modalOverlay__coinbaseOverlay__bodyWrapper__body
|--- modalOverlay__coinbaseOverlay__bodyWrapper__body__iframe
|- modalOverlay__coinbaseOverlay__footer

dashboardFooter
dashboardFooter_coinbaseFooter
|- dashboardFooter_coinbaseFooter__coinbaseMessageWrapper
|-- dashboardFooter_coinbaseFooter__coinbaseMessageWrapper__message

Add bitcoinPanel
Add iconWrapperWidth

Remove modalOverlay.less (Addresses #8661)
Remove data-l10n-id='bitcoinAddress' in favor of 'bitcoinPaymentURL' (Closes #8662)
Remove styles.deviderFirst
Remove styles.alignMiddle
Remove styles.dialogInsideMargin__bottom
Remove styles.disabledPanel
Remove bitcoinDashboard class
Remove unnecessary div
Remove margin:0 from settingListTitle (panel__divider_left__listTitleWrapper__title)

Also:
- Refactor qrcodeOverlay
- Edit qrcodeOverlayContent
- Replace fa icons with functions
- Replace padding magical number with `panelPadding` and `panelPaddingBody`
- Fix done button height on Add funds dialog
- Align done button if coinbase logo is displayed (Closes #6372)
- Display title on Brave wallet QR code (Closes #8663)

Test Plan:
1. Open about:preferences#payments
2. Click "Add funds"
3. Make sure the 3 icons inside the dialog body is aligned

Test plan
1. Open about:preferences#payments
2. Click "Add funds"
3. Click "Fund with credit/debit"
4. Make sure the coinbase widget is displayed at the center

Test Plan:
1. Add your country code to coinbaseCountries.js
2. Open about:preferences#payments
3. Click "Add funds"
4. Click "Display QR code"
5. Make sure the 3 icons appear on the footer

Test Plan:
1. Open about:preferences#payments
2. Enable payments
3. Click "Add funds"
4. Click "Display QR Code"
5. Make sure both the QR image and the label is aligned at the center

Test Plan:
1. Enable coinbase widget (via VPN)
2. Open about:preferences#payments
3. Click "Add funds"
4. Make sure "Done" button is aligned with the other buttons
5. Disable the coinbase widget
6. Make sure "Done button" is aligned to the right side

Test Plan:
1. Open about:preferences#payments
2. Click "Add funds"
3. Click "Display QR code"
4. Hover on the QR code image
5. Make sure its title is displayed
payment modal dialog - replace divs with sections
b732f53 
- advancedSettings.js
- bitcoinDashboard.js
- disabledContent.js
- enabledContent.js
- history.js
- ledgerBackup.js
- ledgerRecovery.js
- ledgerTable.js
Polish modal dialog - syncTab.js
229ff44 
- Add commonStyles.noMarginBottom to the last element of newOverlayContent
- Remove syncOverlayBody__wrapper
- Remove className='syncOverlay'
- Fix sync passphrase selectability
- Fix table width that doesn't make sense
- Replace divs with sections

Test Plan:
1. Open about:preferences#sync
2. Enable Sync
3. Click "Sync a new device"
4. Make sure just padding exists under "Show secret code words"
5. Click "Show secret code words"
6. Make sure the code is selectable
Polish modalOverlay.js
8babd6a 
Addresses #8054

- Replace let with const
- Replace 50px with a variable
- Replace divs with header, section, and footer
- Add registerMock
modal dialog - preferences.less
5aef946 
Addresses #8661

Remove leftover styles under .board
Remove leftover styles under .modal
@luixxiul luixxiul added polish Nice to have — usually related to front-end/visual tasks. refactoring/aphrodite labels May 3, 2017
@luixxiul luixxiul self-assigned this May 3, 2017
@luixxiul
Copy link
Contributor Author

luixxiul commented May 3, 2017 •
edited
Loading

@cezaraugusto - asking for review the code.
@srirambv - asking for review that manual test plans would work.

If you guys think it's good to go, I'll merge the commits. Thanks in advance 😄

@luixxiul luixxiul added this to the 0.15.3 milestone May 4, 2017
cezaraugusto
cezaraugusto approved these changes May 4, 2017
View reviewed changes
Copy link
Contributor

@cezaraugusto cezaraugusto left a comment

Choose a reason for hiding this comment

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

LGTM great job

@bsclifton bsclifton merged commit 81ee54b into brave:master May 4, 2017
@srirambv srirambv mentioned this pull request May 4, 2017
Closed
This was referenced May 4, 2017
Closed
Closed
Closed
@luixxiul luixxiul deleted the modalOverlay-refactoring-for-PR branch May 5, 2017 00:30
This was referenced May 5, 2017
Merged
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cezaraugusto cezaraugusto cezaraugusto approved these changes

@srirambv srirambv Awaiting requested review from srirambv

Assignees

@luixxiul luixxiul

Labels
polish Nice to have — usually related to front-end/visual tasks. QA/test-plan-specified refactoring/aphrodite release-notes/include
Projects
None yet
Milestone
0.15.300 (Frozen, only critical adds ...
Development

Successfully merging this pull request may close these issues.
3 participants
@luixxiul @cezaraugusto @bsclifton