-
Notifications
You must be signed in to change notification settings - Fork 975
Update advancedSettings.js to the modified BEM style #10340
Update advancedSettings.js to the modified BEM style #10340
Conversation
@@ -56,29 +56,30 @@ class AdvancedSettingsContent extends ImmutableComponent { | |||
</SettingItem> | |||
</SettingsList> | |||
</div> | |||
<div className={css(styles.settingsPanelDivider, styles.lastDivider)}> | |||
<SettingsList className={css(commonStyles.noMarginBottom)} | |||
listClassName={css(styles.list)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we just don't need that
flexWrap: 'nowrap' | ||
display: 'grid', | ||
gridTemplateColumns: '1fr .75fr', | ||
gridColumnGap: '32px' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
magic numbers
Codecov Report
@@ Coverage Diff @@
## master #10340 +/- ##
=======================================
Coverage 54.33% 54.33%
=======================================
Files 245 245
Lines 21156 21156
Branches 3260 3260
=======================================
Hits 11495 11495
Misses 9661 9661
|
Closes #10339 Also: - Replace flex with grid - aphrodite -> aphrodite/no-important Auditors: @cezaraugusto Test Plan: 1. Open about:preferences#payments 2. Enable Payments 3. Click the advanced setting icon 4. Make sure the things are properly placed
@@ -3,7 +3,7 @@ | |||
* You can obtain one at http://mozilla.org/MPL/2.0/. */ | |||
|
|||
const React = require('react') | |||
const {StyleSheet, css} = require('aphrodite') | |||
const {StyleSheet, css} = require('aphrodite/no-important') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<3
settingsPanelDivider: { | ||
width: '100%' | ||
display: 'grid', | ||
gridTemplateColumns: '1fr .75fr', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cutting-edge!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ looks great. Please check this on Windows too with 125% DPI if you didn't already just to check it looks good. Nice job!
Closes #10339
Also:
Test Plan:
Updated screenshot:
Old screenshot
Submitter Checklist:
git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist:
Tests