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

Refactor payment/history.js #8038

Merged

luixxiul merged 2 commits into brave:master from luixxiul:paymentHistory-refactoring

Apr 16, 2017

Contributor

luixxiul commented Apr 2, 2017 •

edited

Loading

See: cropped picture of the 6th commit on #6047 (comment)

Closes #8037

Changed aphrodite to aphrodite/no-important (to speed up removing the LESS files)
Aphrodite without no-important overwrites the existing code, which should have been removed from the LESS files at the same time. It will make it difficult to know which styles should be kept and removed, having confidence in that changes would not cause regressions. It might be slowing down the refactoring work.
Modified modalOverlay.js by adding customHeaderClassesStr
The intent of the change is to make it possible to apply custom styles to sectionTitle. In this case, braveMediumOrange to the title.
Introduced paymentStylesVariables and paymentHistoryTablePadding
Appied it to leftRow and paymentHistoryOverlay__title
It aligns the padding-left of sectionTitle and the most left row.
Set position:sticky to the table header
It always display the table header. Sticky has been available on chromium since a couple of versions.
Set columnHeight and columnPadding
Reverted commonStyles.primaryButton to primaryButton temporarily
There have been already browserButton and primaryButton available with commonStyles.js, still these styles are overwritten by ones which button.less defines. We have to work on refactoring button.less at first.

Also:

Removed medium
Removed pull-left, which is no longer used

Auditors:

Test Plan:

Open about:preferences#payments
The modal dialog looks like the cropped picture of the 6th commit on Refactor about:preferences#payments followups #6047 (comment)

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

Test Plan:

luixxiul added feature/rewards refactoring labels

luixxiul requested review from cezaraugusto and NejcZdovc

April 2, 2017 10:57

luixxiul added the PR/needs-QA-attention ☕ label

Contributor Author

luixxiul commented Apr 2, 2017

#6047 (comment)

Currently:

With this PR:

luixxiul requested a review from bradleyrichter

April 2, 2017 11:01

luixxiul self-assigned this

NejcZdovc reviewed

View reviewed changes

app/renderer/components/preferences/payment/history.js Outdated

    
                    <Button l10nId='paymentHistoryOKText'

                      className={css(commonStyles.primaryButton)}

                      className='primaryButton'

Contributor

NejcZdovc Apr 2, 2017

Why did you change this back to regular class?

Contributor Author

luixxiul Apr 2, 2017 •

edited

Loading

As I wrote in the commit message:

Reverted commonStyles.primaryButton to primaryButton temporarily

There have been already browserButton and primaryButton available with commonStyles.js, still these styles are overwritten by ones which button.less defines. We have to work on refactoring button.less at first.

It is because aphrodite was changed to aphrodite/no-important. I left the reasoning in the commit message on this change too.

Contributor

NejcZdovc Apr 2, 2017

Sorry, I didn't read you comment in the commit. Make sense, so let's create an issue for this and track all things that need's to be done there.

Contributor Author

luixxiul Apr 2, 2017

+1!!

This was referenced Apr 2, 2017

Refactor syncTab.js with Aphrodite #8044

Merged

Refactor payment/history.js #8037

Closed

luixxiul added PR/work-in-progress ⚒ PR/needs-QA-attention ☕ and removed PR/needs-QA-attention ☕ PR/work-in-progress ⚒ labels

cezaraugusto reviewed

View reviewed changes

app/renderer/components/preferences/paymentsTab.js Outdated

		const globalStyles = require('../styles/global')
		<<<<<<< HEAD

Contributor

cezaraugusto Apr 6, 2017

I think we don't want it ;)

cezaraugusto reviewed

View reviewed changes

app/renderer/components/preferences/paymentsTab.js Outdated

               const {paymentStyles} = require('../styles/payment')
               const settingIcon = require('../../../extensions/brave/img/ledger/icon_settings.svg')
+              =======

Contributor

cezaraugusto Apr 6, 2017

(2)

cezaraugusto reviewed

View reviewed changes

app/renderer/components/preferences/paymentsTab.js Outdated

+              =======
+              const {paymentStyles, paymentStylesVariables} = require('../styles/payment')
+              const advanceIcon = require('../../../extensions/brave/img/ledger/icon_settings.svg')
+              >>>>>>> Refactor payment/history.js

Contributor

cezaraugusto Apr 6, 2017

(3)

cezaraugusto reviewed

View reviewed changes

app/renderer/components/preferences/paymentsTab.js Outdated

@@ @@ -364,6 +385,21 @@ const styles = StyleSheet.create({ @@
                   ':hover': {
                     backgroundColor: globalStyles.color.chromeTertiary
                   }
+              <<<<<<< HEAD

Contributor

cezaraugusto Apr 6, 2017

:)

cezaraugusto reviewed

View reviewed changes

app/renderer/components/preferences/paymentsTab.js Outdated

@@ @@ -160,8 +166,12 @@ class PaymentsTab extends ImmutableComponent { @@
                       />
                       : null
                     }
+              <<<<<<< HEAD

Contributor

cezaraugusto Apr 6, 2017

same

cezaraugusto reviewed

View reviewed changes

app/renderer/components/preferences/paymentsTab.js Outdated

@@ @@ -276,12 +286,23 @@ const styles = StyleSheet.create({ @@
                 switchWrap: {
                   width: paymentStyles.width.tableCell
                 },
+              <<<<<<< HEAD

Contributor

cezaraugusto Apr 6, 2017

(._.)

cezaraugusto reviewed

View reviewed changes

app/renderer/components/preferences/paymentsTab.js Outdated

                 switchWrap__switchControl: {
                   // TODO: Refactor switchControls.less
                   paddingTop: '0 !important',
                   paddingBottom: '0 !important'
                 },
                 switchWrap__switchSpan: {
+              =======

Contributor

cezaraugusto Apr 6, 2017

( .-. )

cezaraugusto reviewed

View reviewed changes

app/renderer/components/preferences/paymentsTab.js Outdated

+                },
+                switchSpan: {
+              >>>>>>> Refactor payment/history.js

Contributor

cezaraugusto Apr 6, 2017

(._.)

cezaraugusto reviewed

View reviewed changes

app/renderer/components/preferences/paymentsTab.js Outdated

+                  color: `${globalStyles.color.braveMediumOrange} !important`,
+                  padding: `25px 0 25px ${paymentStylesVariables.spacing.paymentHistoryTablePadding} !important`,
+                  textIndent: '0 !important'
+              >>>>>>> Refactor payment/history.js

Contributor

cezaraugusto Apr 6, 2017

(｡◕‿‿◕｡)

cezaraugusto reviewed

View reviewed changes

app/renderer/components/preferences/paymentsTab.js Outdated

                     <div className={css(styles.flexAlignEnd)}>
+              =======
+                    <div className={css(styles.titleBar)}>
+              >>>>>>> Refactor payment/history.js

Contributor

cezaraugusto Apr 6, 2017

(｡◕‿◕｡)

luixxiul changed the base branch from master to refactoring-aphrodite

April 6, 2017 09:21

luixxiul requested a review from NejcZdovc

April 6, 2017 15:36

NejcZdovc reviewed

View reviewed changes

app/renderer/components/preferences/payment/history.js Outdated

+                  height: columnHeight,
+                  alignItems: 'center',
+                  cursor: 'default',
+                  WebkitUserSelect: 'none'

Contributor

NejcZdovc Apr 7, 2017

let's use basic user select and not a webkit

Contributor Author

luixxiul Apr 8, 2017

Updated: ae31c14#diff-d7a7d22ddb5b69ebc41381e4864ff9d0R166

luixxiul mentioned this pull request

Replace -webkit-user-select with user-select #8124

Closed

Contributor Author

luixxiul commented Apr 9, 2017

pinging @bradleyrichter for design review :-)

luixxiul changed the base branch from refactoring-aphrodite to master

April 10, 2017 03:19

Suguru Hirahara added 2 commits

April 10, 2017 12:24


          Refactor payment/history.js

304469a

See: cropped picture of the 6th commit on #6047 (comment)

Closes #8037

- Changed aphrodite to aphrodite/no-important (to speed up removing the LESS files)
  Aphrodite without no-important overwrites the existing code, which should have been removed from the LESS files at the same time. It will make it difficult to know which styles should be kept and removed, having confidence in that changes would not cause regressions. It might be slowing down the refactoring work.

- Modified modalOverlay.js by adding customHeaderClassesStr
  The intent of the change is to make it possible to apply custom styles to the sectionTitle. In this case, braveMediumOrange to the title.

- Introduced paymentStylesVariables and paymentHistoryTablePadding
- Appied it to leftRow and paymentHistoryOverlay__title
  It aligns the padding-left of the sectionTitle and the most left row.

- Set position:sticky to the table header
  It always display the table header. Sticky has been available on chromium since a couple of versions.

- Set columnHeight and columnPadding

- Reverted commonStyles.primaryButton to primaryButton temporarily
  There have been already browserButton and primaryButton available with commonStyles.js, still these styles are overwritten by ones which button.less defines. We have to work on refactoring button.less at first.

Also:
- Removed medium
- Removed pull-left, which is no longer used

Auditors:

Test Plan:
1. Run npm run add-simulated-payment-history
2. Open about:preferences#payments
3. The modal dialog looks like the cropped picture of the 6th commit on #6047 (comment)


          Partial refactoring of modalOverlay

101de8f

Addresses #8054

- Renamed customTitleClasses to customDialogClasses
- Renamed customHeaderClasses to customTitleClasses

customeDialogClasses
 |-- customTitleClasses

Auditors:

Test Plan:
1. Open about:preferences#payments
2. Enable payments
3. Click "Add funds"
4. Make sure the style of modal overlay dialog is not broken
5. Click "Display QR"
6. Make sure the QR overlay is displayed
7. Close those overlay dialogs
8. Click payment history icon
9. Make sure the style of the modal dialog is not broken
10. Close the dialog
11. Click advanced options icon
12. Click "Backup your wallet"
13. Make sure background color of the dialog is not changed

luixxiul modified the milestones: 0.14.2, 0.14.3

luixxiul requested review from bsclifton and cezaraugusto

April 11, 2017 07:52

luixxiul mentioned this pull request

Create sectionTitle.js #8237

Merged

4 tasks

luixxiul referenced this pull request


          Update style.md

234cfba

- Auditors: @bsclifton, @luixxiul
- Close #8333
Test Plan: styles.md should have guidelines about how we make use of Aphrodite

cezaraugusto approved these changes

View reviewed changes

Contributor

cezaraugusto left a comment

LGTM

luixxiul merged commit 24fafe1 into brave:master

Contributor Author

luixxiul commented Apr 16, 2017

@cezaraugusto thanks for reviewing.

luixxiul deleted the paymentHistory-refactoring branch

April 16, 2017 06:47

luixxiul removed the PR/needs-QA-attention ☕ label

Contributor Author

luixxiul commented Apr 16, 2017 •

edited

Loading

@bsclifton please change the milestone if this is going to be merged to 0.14.2, thanks.

#8037 as well.

luixxiul removed the request for review from bsclifton

April 16, 2017 07:10

luixxiul mentioned this pull request

prevent Dialog from being uncloseable with long text #8475

Merged

4 tasks

Contributor Author

luixxiul commented May 1, 2017

FYI: there are several reports that position:sticky does not work on chromium 58. See https://developers.google.com/web/updates/2016/12/position-sticky for the comments.

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

Labels

feature/rewards refactoring