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

Commit

Permalink
Add browserButton_subtleItem and fix regressions
Browse files Browse the repository at this point in the history 
Closes #9072
Addresses #9071

Also:
- Add lines in the same way as LESS files
- Add comments for refenrence (We could remove them after refactoring work)
- Fix regressions on browserButton_default
  - Add globalStyles.button.color to replace it with globalStyles.button.default.color
- Fix a typo

Auditors:

Test Plan 1:
(0. Take a screenshot of the buttons section on about:styles before testing the PR for later use)
1. Open about:styles
2. Click "buttons"
3. Take another screenshot of the section
4. Compare the two screenshots and make sure they look same

Test Plan 2:
1. Open about:styles
2. Toggle developer tools
3. Make sure button's padding is set to '5px 16px'

Test Plan 3:
1. Open downloadme.org
2. Click "Advanced" button
3. Make sure the button is not pushed down
4. Click "Hide advanced" button
5. Make sure the button does not move
  • Loading branch information
@cezaraugusto
Suguru Hirahara authored and cezaraugusto committed May 30, 2017
1 parent ce6613c commit f3c503e
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 18 deletions.
65 changes: 55 additions & 10 deletions app/renderer/components/common/browserButton.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ class BrowserButton extends ImmutableComponent {
styles.browserButton,
this.props.primaryColor && [styles.browserButton_default, styles.browserButton_primaryColor],
this.props.secondaryColor && [styles.browserButton_default, styles.browserButton_secondaryColor],
this.props.subtleItem && [styles.browserButton_default, styles.browserButton_subtleItem],
this.props.extensionItem && styles.browserButton_extensionItem,
this.props.groupedItem && styles.browserButton_groupedItem,
this.props.notificationItem && styles.browserButton_notificationItem
// TODO: These are other button styles app-wise
// that needs to be refactored and included in this file
// .............................................
// this.props.smallItem && styles.browserButton_actionItem,
// this.props.smallItem && styles.browserButton_smallItem,
// this.props.actionItem && styles.browserButton_actionItem,
// this.props.navItem && styles.browserButton_navItem,
// this.props.panelItem && styles.browserButton_panelItem,
// this.props.subtleItem && styles.browserButton_subtleItem
]
}
render () {
Expand All @@ -46,18 +46,19 @@ class BrowserButton extends ImmutableComponent {
}
}

const buttonSize = '25px'

const styles = StyleSheet.create({
browserButton: {
margin: '0 3px',
whiteSpace: 'nowrap',
outline: 'none',
cursor: 'default',
display: 'inline-block',
lineHeight: '25px',
height: '25px',
width: '25px',
lineHeight: buttonSize,
height: buttonSize,
width: buttonSize,
fontSize: '13px',
color: globalStyles.button.default.color,
borderRadius: '2px',
textAlign: 'center',
transition: '.1s opacity, .1s background',
Expand All @@ -68,49 +69,71 @@ const styles = StyleSheet.create({
backgroundImage: 'none',
backgroundColor: globalStyles.button.default.backgroundColor,
border: 'none',

// cf: https://github.com/brave/browser-laptop/blob/548e11b1c889332fadb379237555625ad2a3c845/less/button.less#L49
color: globalStyles.button.color,

':hover': {
color: globalStyles.button.default.hoverColor
}
},

// applies for primary and white buttons
browserButton_default: {
// cf: https://github.com/brave/browser-laptop/blob/548e11b1c889332fadb379237555625ad2a3c845/less/button.less#L92
color: globalStyles.button.default.color,

position: 'relative',
boxShadow: globalStyles.button.default.boxShadow,
cursor: 'pointer',
lineHeight: 1.25,
width: 'auto',
height: 'auto',
minWidth: '78px',
fontSize: '13px',
padding: '7px 20px',
fontSize: globalStyles.spacing.defaultFontSize,

// cf: https://github.com/brave/browser-laptop/blob/548e11b1c889332fadb379237555625ad2a3c845/less/button.less#L94-L95
paddingTop: '5px',
paddingBottom: '5px',

// cf: https://github.com/brave/browser-laptop/blob/548e11b1c889332fadb379237555625ad2a3c845/less/button.less#L105-L106
paddingRight: '16px',
paddingLeft: '16px',

// cf: https://github.com/brave/browser-laptop/blob/548e11b1c889332fadb379237555625ad2a3c845/less/button.less#L98
minWidth: `calc(${globalStyles.spacing.defaultFontSize} * 6)`, // issue #6384

':active': {
// push the button down when active
bottom: '-1px'
}
},

browserButton_primaryColor: {
background: globalStyles.button.primary.background,
borderLeft: '2px solid transparent',
borderRight: '2px solid transparent',
borderTop: `2px solid ${globalStyles.button.primary.gradientColor1}`,
borderBottom: `2px solid ${globalStyles.button.primary.gradientColor2}`,
color: globalStyles.button.primary.color,
cursor: 'pointer',

':hover': {
border: `2px solid ${globalStyles.button.primary.borderHoverColor}`,
color: globalStyles.button.primary.hoverColor
}
},

browserButton_secondaryColor: {
background: globalStyles.button.secondary.background,
border: '1px solid white',
color: globalStyles.button.secondary.color,
cursor: 'pointer',

':hover': {
border: `1px solid ${globalStyles.button.secondary.borderHoverColor}`,
color: globalStyles.button.secondary.hoverColor
}
},

browserButton_extensionItem: {
WebkitAppRegion: 'no-drag',
backgroundSize: 'contain',
Expand All @@ -119,20 +142,42 @@ const styles = StyleSheet.create({
opacity: '0.85',
backgroundRepeat: 'no-repeat'
},

browserButton_groupedItem: {
// Legacy LESS inside ledger is too nested
// and this style won't have effect without using !important
//
// TODO: remove !important and check advancedSettings.js
// once preferences is fully refactored
marginRight: '4px !important',
marginLeft: '4px !important'
},

browserButton_notificationItem: {
fontSize: '13px',
marginRight: '10px',
padding: '2px 15px',
textTransform: 'capitalize',
width: 'auto'
},

browserButton_subtleItem: {
background: globalStyles.button.subtle.backgroundColor,

// 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
// 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
':active': {
bottom: 0
}
}
})

Expand Down
14 changes: 12 additions & 2 deletions app/renderer/components/styles/global.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,30 +252,40 @@ const globalStyles = {
animationFillMode: 'forwards'
}
},

button: {
color: '#5a5a5a',

default: {
color: '#5a5a5a',
color: '#fff',
backgroundColor: 'transparent',
hoverColor: '#000',
boxShadow: '0px 1px 5px -1px rgba(0, 0, 0, 0.5)'
},

primary: {
gradientColor1: '#FF7A1D',
gradientColor2: '#ff5000',
background: 'linear-gradient(#FF7A1D, #ff5000)',
color: '#fff',
hoverColor: '#fff',
borderHoverColor: '#fff'
},

secondary: {
gradientColor1: '#fff',
gradientColor2: '#ececec',
background: 'linear-gradient(#fff, #ececec)',
color: '#444',
hoverColor: '#000',
borderHoverColor: 'rgb(153, 153, 153)'
},

subtle: {
// cf: https://github.com/brave/browser-laptop/blob/548e11b1c889332fadb379237555625ad2a3c845/less/button.less#L151
backgroundColor: '#ccc'
}
},

braveryPanel: {
color: '#3b3b3b',

Expand Down
6 changes: 3 additions & 3 deletions js/about/safebrowsing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */

const React = require('react')
const Button = require('../../app/renderer/components/common/button')
const BrowserButton = require('../../app/renderer/components/common/browserButton')

const {StyleSheet, css} = require('aphrodite/no-important')

Expand Down Expand Up @@ -35,7 +35,7 @@ class SafebrowsingPage extends React.Component {
{this.state.advanced
? <section>
<div className='buttons'>
<Button className='subtleButton'
<BrowserButton subtleItem
l10nId='safebrowsingErrorHideAdvanced'
testId='safebrowsingErrorHideAdvanced'
onClick={this.onAdvancedToggle.bind(this)}
Expand All @@ -47,7 +47,7 @@ class SafebrowsingPage extends React.Component {
</div>
</section>
: <section className='buttons'>
<Button className='subtleButton'
<BrowserButton subtleItem
l10nId='safebrowsingErrorAdvanced'
testId='safebrowsingErrorAdvanced'
onClick={this.onAdvancedToggle.bind(this)}
Expand Down
5 changes: 2 additions & 3 deletions js/about/styles.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,9 @@ class AboutStyle extends ImmutableComponent {
onClick={'{this.onRemoveBookmark}'} />
</Code></Pre>

<button data-l10n-id='subtleButton' className='browserButton subtleButton' onClick={this.onRemoveBookmark} />
<BrowserButton subtleItem l10nId='subtleButton' onClick={this.onRemoveBookmark} />
<Pre><Code>
&lt;button data-l10n-id='cancel' className='browserButton subtleButton'{'\n'}
onClick={'{this.onRemoveBookmark}'} />
&lt;BrowserButton subtleItem l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />
</Code></Pre>

<BrowserButton primaryColor groupedItem l10nId='primaryButton' onClick={this.onRemoveBookmark} />
Expand Down

0 comments on commit f3c503e

Please sign in to comment.