From 9e730c5bf84facbbc7ba723d24dcc1c395c90b40 Mon Sep 17 00:00:00 2001 From: Cezar Augusto Date: Fri, 13 Oct 2017 15:19:53 -0300 Subject: [PATCH] Merge pull request #11290 from luixxiul/polish-browserButton-groupedItem Cancel the margin-left of browserButton with groupedItem --- .../components/common/browserButton.js | 41 +++++++++++-------- js/about/styles.js | 28 ++++++++----- 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/app/renderer/components/common/browserButton.js b/app/renderer/components/common/browserButton.js index 5ec1c959702..739911fc1c8 100644 --- a/app/renderer/components/common/browserButton.js +++ b/app/renderer/components/common/browserButton.js @@ -108,8 +108,14 @@ const styles = StyleSheet.create({ // The buttons as such do not require margin. // They are expected to have margin only if they are grouped. - // Also see note on browserButton_groupedItem below. - margin: 0, + // + // Avoid using shorthand properties to override the values + // with browserButton_groupedItem below, without declaring !important. + // See: https://github.com/Khan/aphrodite#object-key-ordering + marginTop: 0, + marginRight: 0, + marginBottom: 0, + marginLeft: 0, // cf: https://github.com/brave/browser-laptop/blob/548e11b1c889332fadb379237555625ad2a3c845/less/button.less#L49 color: globalStyles.button.color, @@ -155,7 +161,7 @@ const styles = StyleSheet.create({ browserButton_fitContent: { // See: 11021 - // Ensure that the button label does not overflow + // Ensure that the button label does not overflow. width: `calc(${globalStyles.spacing.defaultFontSize} * 6)`, // issue #6384 minWidth: 'fit-content' }, @@ -199,22 +205,23 @@ const styles = StyleSheet.create({ }, browserButton_groupedItem: { - // Legacy LESS inside ledger is too nested - // and this style won't have effect without using !important + // Issue #9252 + // Because the grouped buttons are *by design* aligned to the right + // with flex/grid, margin-right of each one should be set to zero. + // If you set margin-right to those buttons, it is expected to lead to style inconsistency. // - // TODO: remove !important and check advancedSettings.js - // once preferences is fully refactored + // Before making a change, please consult with Brad on your proposal. + marginRight: 0, // 7.5px = 1/12 inch - marginLeft: '7.5px !important', + marginLeft: '7.5px', - // Issue #9252 - // Because the grouped buttons are *by design* aligned to the right - // with flex/grid, margin-right of each one should be null. - // If you set margin-right to those buttons, it is expected to lead - // to the alignment inconsistency. - // Before making a change, please consult with Brad on your proposal. - marginRight: 0 + // Issue #11288 + // Cancel the margin-left of the first element. + // The buttons have to have a parent. Beginning with Selectors Level 4, that is no longer required. + ':first-child': { + marginLeft: '0' + } }, browserButton_notificationItem: { @@ -228,14 +235,14 @@ const styles = StyleSheet.create({ // cf: https://github.com/brave/browser-laptop/blob/548e11b1c889332fadb379237555625ad2a3c845/less/button.less#L152 fontSize: '14px', - // Adding box-shadow:none to cancel the box-shadow specified on browserButton_default + // Adding box-shadow:none to cancel the box-shadow specified on browserButton_default. // On button.less, box-shadow is only speficied to .primaryButton and .whiteButton // cf: // https://github.com/brave/browser-laptop/blob/548e11b1c889332fadb379237555625ad2a3c845/less/button.less#L114 // https://github.com/brave/browser-laptop/blob/548e11b1c889332fadb379237555625ad2a3c845/less/button.less#L137 boxShadow: 'none', - // Cancel pushing down the button to make the button subtle + // Cancel pushing down the button to make the button subtle. ':active': { bottom: 0 } diff --git a/js/about/styles.js b/js/about/styles.js index ee76fc25059..c9aabdf8f49 100644 --- a/js/about/styles.js +++ b/js/about/styles.js @@ -290,13 +290,17 @@ class AboutStyle extends ImmutableComponent { <BrowserButton subtleItem l10nId='cancel' onClick={'{this.onRemoveBookmark}'} /> - - - +
+ + + + 

-          <BrowserButton groupedItem primaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />{'\n'}
-          <BrowserButton groupedItem secondaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />{'\n'}
-          <BrowserButton groupedItem primaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />
+          <div>
+          <BrowserButton groupedItem primaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />{'\n'}
+          <BrowserButton groupedItem secondaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />{'\n'}
+          <BrowserButton groupedItem primaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />
+          </div>
@@ -304,11 +308,15 @@ class AboutStyle extends ImmutableComponent { <BrowserButton extensionItem l10nId='cancel' onClick={'{this.onRemoveBookmark}'} /> - - +
+ + + 

-          <BrowserButton groupedItem secondaryColor notificationItem l10nId='Allow' onClick={'{this.onEnableAutoplay}'} />{'\n'}
-          <BrowserButton groupedItem secondaryColor notificationItem l10nId='Deny' onClick={'{this.onEnableAutoplay}'} />
+          <div>
+          <BrowserButton groupedItem secondaryColor notificationItem l10nId='Allow' onClick={'{this.onEnableAutoplay}'} />{'\n'}
+          <BrowserButton groupedItem secondaryColor notificationItem l10nId='Deny' onClick={'{this.onEnableAutoplay}'} />
+          </div>