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

Increase pinned tabs width #10533

Merged
merged 1 commit into from
Sep 5, 2017
Merged

Increase pinned tabs width #10533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions app/renderer/components/styles/global.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,6 @@ globalStyles.color.loadTimeColor = globalStyles.color.highlightBlue
globalStyles.color.activeTabDefaultColor = globalStyles.color.chromePrimary
globalStyles.color.switchBG_on = globalStyles.color.braveOrange

globalStyles.spacing.tabHeight = globalStyles.spacing.tabsToolbarHeight

globalStyles.braveryPanel.stats.colorAds = globalStyles.color.statsRed
globalStyles.braveryPanel.stats.colorRedirected = globalStyles.color.statsBlue
globalStyles.braveryPanel.stats.colorFp = globalStyles.color.statsYellow
Expand Down
39 changes: 23 additions & 16 deletions app/renderer/components/styles/tab.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const styles = StyleSheet.create({
boxSizing: 'border-box',
color: '#5a5a5a',
display: 'flex',
height: globalStyles.spacing.tabHeight,
marginTop: '0',
transition: `transform 200ms ease, ${globalStyles.transition.tabBackgroundTransition}`,
left: '0',
Expand All @@ -29,6 +28,10 @@ const styles = StyleSheet.create({
padding: globalStyles.spacing.defaultTabPadding,
position: 'relative',

// globalStyles.spacing.tabHeight has been set to globalStyles.spacing.tabsToolbarHeight,
// which is 1px extra due to the border-top of .tabsToolbar
height: '-webkit-fill-available',

':hover': {
background: 'linear-gradient(to bottom, rgba(255, 255, 255, 0.8), rgba(250, 250, 250, 0.4))'
}
Expand Down Expand Up @@ -68,12 +71,6 @@ const styles = StyleSheet.create({
overflow: 'hidden'
},

// Add extra space for pages that have no icon
// such as about:blank and about:newtab
noFavicon: {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not called anywhere.

padding: '0 6px'
},

alternativePlayIndicator: {
borderTop: '2px solid lightskyblue'
},
Expand All @@ -83,24 +80,28 @@ const styles = StyleSheet.create({
alignItems: 'center',
display: 'flex',
flex: '1',
minWidth: '0', // @see https://bugzilla.mozilla.org/show_bug.cgi?id=1108514#c5

// @see https://bugzilla.mozilla.org/show_bug.cgi?id=1108514#c5
minWidth: '0',

// prevent the icons wrapper from being the target of mouse events.
pointerEvents: 'none'
},

isPinned: {
paddingLeft: globalStyles.spacing.defaultIconPadding,
paddingRight: globalStyles.spacing.defaultIconPadding
padding: 0,
width: `calc(${globalStyles.spacing.tabsToolbarHeight} * 1.1)`,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

26px * 1.1 is nearly equal to the pinned tab width where the padding is increased from 2px to 4px.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems fragile to me. What if at a given point we decide to update tabs toolbar height?

pinned tabs sizing are also are not used anywhere else so I won't mind having the size explicitly described here.

justifyContent: 'center'
},

active: {
background: `rgba(255, 255, 255, 1.0)`,
height: globalStyles.spacing.tabHeight,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant

marginTop: '0',
borderWidth: '0 1px 0 0',
borderStyle: 'solid',
borderColor: '#bbb',
color: '#000',

':hover': {
background: `linear-gradient(to bottom, #fff, ${globalStyles.color.chromePrimary})`
}
Expand Down Expand Up @@ -138,13 +139,19 @@ const styles = StyleSheet.create({
fontSize: globalStyles.fontSize.tabIcon,
backgroundPosition: 'center',
backgroundRepeat: 'no-repeat',
display: 'flex',
alignSelf: 'center',
position: 'relative',
textAlign: 'center',
justifyContent: 'center',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need those.

paddingLeft: globalStyles.spacing.defaultIconPadding,
paddingRight: globalStyles.spacing.defaultIconPadding
},

icon_audio: {
color: globalStyles.color.highlightBlue,

// 16px
fontSize: `calc(${globalStyles.fontSize.tabIcon} + 2px)`,

// equal spacing around audio icon (favicon and tabTitle)
padding: globalStyles.spacing.defaultTabPadding,
paddingRight: '0 !important'
}
})

Expand Down
14 changes: 4 additions & 10 deletions app/renderer/components/tabs/content/audioTabIcon.js
View file
Open in desktop
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 {StyleSheet, css} = require('aphrodite/no-important')
const {css} = require('aphrodite/no-important')
const Immutable = require('immutable')

// Components
Expand Down Expand Up @@ -61,17 +61,11 @@ class AudioTabIcon extends React.Component {

render () {
return <TabIcon
className={css(tabStyles.icon, styles.audioIcon)}
className={css(tabStyles.icon, tabStyles.icon_audio)}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifying a style in each file will create style inconsistency quite easily.

symbol={this.audioIcon}
onClick={this.toggleMute} />
onClick={this.toggleMute}
/>
}
}

module.exports = ReduxComponent.connect(AudioTabIcon)

const styles = StyleSheet.create({
audioIcon: {
color: globalStyles.color.highlightBlue,
fontSize: '16px'
}
})
7 changes: 3 additions & 4 deletions app/renderer/components/tabs/content/tabIcon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@ const globalStyles = require('../../styles/global')
class TabIcon extends ImmutableComponent {
render () {
const styles = StyleSheet.create({
icon: {
tabIcon: {
Copy link
Contributor Author

@luixxiul luixxiul Aug 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced as it is confusing on debugging (we already have icon on styles/tab.js).

fontSize: this.props.symbolContent ? '8px' : 'inherit',
display: 'flex',
alignSelf: 'center',
width: globalStyles.spacing.iconSize,
height: globalStyles.spacing.iconSize,
alignItems: 'center',
justifyContent: this.props.symbolContent ? 'flex-end' : 'left',
justifyContent: this.props.symbolContent ? 'flex-end' : 'center',
fontWeight: this.props.symbolContent ? 'bold' : 'normal',
color: this.props.symbolContent ? globalStyles.color.black100 : 'inherit'
}
Expand All @@ -51,7 +50,7 @@ class TabIcon extends ImmutableComponent {
? <span
className={cx({
[this.props.symbol]: true,
[css(styles.icon)]: true
[css(styles.tabIcon)]: true
})}
data-test-id={this.props['data-test-id']}
data-test2-id={this.props['data-test2-id']}
Expand Down