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

Commit

Permalink
Merge pull request #11290 from luixxiul/polish-browserButton-groupedItem
Browse files Browse the repository at this point in the history 
Cancel the margin-left of browserButton with groupedItem
  • Loading branch information
@cezaraugusto
cezaraugusto committed Oct 13, 2017
1 parent 4b350ed commit 9e730c5
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 27 deletions.
41 changes: 24 additions & 17 deletions app/renderer/components/common/browserButton.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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'
},
Expand Down Expand Up @@ -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: {
Expand All @@ -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
}
Expand Down
28 changes: 18 additions & 10 deletions js/about/styles.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,25 +290,33 @@ class AboutStyle extends ImmutableComponent {
<BrowserButton subtleItem l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />
</Code></Pre>

<BrowserButton groupedItem primaryColor l10nId='primaryColor' onClick={this.onRemoveBookmark} />
<BrowserButton groupedItem secondaryColor l10nId='secondaryColor' onClick={this.onRemoveBookmark} />
<BrowserButton groupedItem primaryColor l10nId='primaryColor' onClick={this.onRemoveBookmark} />
<div>
<BrowserButton groupedItem primaryColor l10nId='primaryColor' onClick={this.onRemoveBookmark} />
<BrowserButton groupedItem secondaryColor l10nId='secondaryColor' onClick={this.onRemoveBookmark} />
<BrowserButton groupedItem primaryColor l10nId='primaryColor' onClick={this.onRemoveBookmark} />
</div>
<Pre><Code>
&lt;BrowserButton groupedItem primaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />{'\n'}
&lt;BrowserButton groupedItem secondaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />{'\n'}
&lt;BrowserButton groupedItem primaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />
&lt;div>
<Tab>&lt;BrowserButton groupedItem primaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />{'\n'}</Tab>
<Tab>&lt;BrowserButton groupedItem secondaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />{'\n'}</Tab>
<Tab>&lt;BrowserButton groupedItem primaryColor l10nId='cancel' onClick={'{this.onRemoveBookmark}'} /></Tab>
&lt;/div>
</Code></Pre>

<BrowserButton extensionItem l10nId='extensionItem' onClick={this.onRemoveBookmark} />
<Pre><Code>
&lt;BrowserButton extensionItem l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />
</Code></Pre>

<BrowserButton groupedItem secondaryColor notificationItem l10nId='notificationItem' onClick={this.onEnableAutoplay} />
<BrowserButton groupedItem secondaryColor notificationItem l10nId='notificationItem' onClick={this.onEnableAutoplay} />
<div>
<BrowserButton groupedItem secondaryColor notificationItem l10nId='notificationItem' onClick={this.onEnableAutoplay} />
<BrowserButton groupedItem secondaryColor notificationItem l10nId='notificationItem' onClick={this.onEnableAutoplay} />
</div>
<Pre><Code>
&lt;BrowserButton groupedItem secondaryColor notificationItem l10nId='Allow' onClick={'{this.onEnableAutoplay}'} />{'\n'}
&lt;BrowserButton groupedItem secondaryColor notificationItem l10nId='Deny' onClick={'{this.onEnableAutoplay}'} />
&lt;div>
<Tab>&lt;BrowserButton groupedItem secondaryColor notificationItem l10nId='Allow' onClick={'{this.onEnableAutoplay}'} />{'\n'}</Tab>
<Tab>&lt;BrowserButton groupedItem secondaryColor notificationItem l10nId='Deny' onClick={'{this.onEnableAutoplay}'} /></Tab>
&lt;/div>
</Code></Pre>

<BrowserButton iconOnly iconClass={globalStyles.appIcons.moreInfo} size='30px' color='rebeccapurple' />
Expand Down

0 comments on commit 9e730c5

Please sign in to comment.