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

Commit

Permalink
UI bug fixing
Browse files Browse the repository at this point in the history
- tabs can now respond beautifully to intersections
- Also BEMify tabs
Auditors: @bsclifton, @luixxiul
fix #6716
fix #7301
fix #7730
fix #7925
fix #10544
fix #10123
fix #10509
fix #10582
fix #10611
  • Loading branch information
cezaraugusto committed Sep 14, 2017
1 parent 633ab0a commit 23fcc08
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 71 deletions.
2 changes: 1 addition & 1 deletion app/renderer/components/styles/commonStyles.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ const styles = StyleSheet.create({
notificationBar__notificationItem: {
backgroundColor: globalStyles.color.notificationItemColor,
boxSizing: 'border-box',
borderTop: `1px solid ${globalStyles.color.tabsToolbarBorderColor}`,
boxShadow: `0 -1px 0 ${globalStyles.color.tabsToolbarBorderColor}`,
lineHeight: '24px',
padding: '8px 20px'
},
Expand Down
3 changes: 2 additions & 1 deletion app/renderer/components/styles/global.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ const globalStyles = {
aboutPageDetailsPageWidth: '704px',
aboutPageSectionPadding: '24px',
aboutPageSectionMargin: '10px',
defaultTabPadding: '0 4px',
defaultTabMargin: '6px',
defaultIconPadding: '2px',
iconSize: '16px',
sessionIconSize: '15px',
Expand Down Expand Up @@ -214,6 +214,7 @@ const globalStyles = {
zindexWindowIsPreview: '1100',
zindexDownloadsBar: '1000',
zindexTabs: '1000',
zindexTabsAudioTopBorder: '10001',
zindexTabsThumbnail: '1100',
zindexTabsDragIndicator: '1100',
zindexNavigationBar: '2000',
Expand Down
148 changes: 83 additions & 65 deletions app/renderer/components/tabs/tab.js
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')
const {StyleSheet, css} = require('aphrodite/no-important')
const Immutable = require('immutable')

// Components
Expand All @@ -23,7 +23,8 @@ const windowActions = require('../../../../js/actions/windowActions')
// Store
const windowStore = require('../../../../js/stores/windowStore')

// State
// State helpers
const privateState = require('../../../common/state/tabContentState/privateState')
const audioState = require('../../../common/state/tabContentState/audioState')
const tabUIState = require('../../../common/state/tabUIState')
const tabState = require('../../../common/state/tabState')
Expand All @@ -33,6 +34,7 @@ const dragTypes = require('../../../../js/constants/dragTypes')

// Styles
const globalStyles = require('../styles/global')
const {theme} = require('../styles/theme')

// Utils
const cx = require('../../../../js/lib/classSet')
Expand Down Expand Up @@ -200,10 +202,10 @@ class Tab extends React.Component {
this.tabNode.addEventListener('auxclick', this.onAuxClick.bind(this))
}

componentDidUpdate () {
componentDidUpdate (prevProps) {
// if a tab was just pinned, unobserve its intersection
// this gives us some more performance boost
if (this.props.isPinnedTab) {
if (prevProps.isPinnedTab !== this.props.isPinnedTab) {
this.observer.unobserve(this.tabSentinel)
}
}
Expand Down Expand Up @@ -234,24 +236,33 @@ class Tab extends React.Component {
mergeProps (state, ownProps) {
const currentWindow = state.get('currentWindow')
const frame = frameStateUtil.getFrameByKey(currentWindow, ownProps.frameKey) || Immutable.Map()
const frameKey = ownProps.frameKey
const tabId = frame.get('tabId', tabState.TAB_ID_NONE)
const isPinned = tabState.isTabPinned(state, tabId)

// TODO: this should have its own method
const notifications = state.get('notifications')
const notificationOrigins = notifications ? notifications.map(bar => bar.get('frameOrigin')) : false
const notificationBarActive = frame.get('location') && notificationOrigins &&
notificationOrigins.includes(UrlUtil.getUrlOrigin(frame.get('location')))
const tabId = frame.get('tabId', tabState.TAB_ID_NONE)

const props = {}
// used in renderer
props.frameKey = ownProps.frameKey
props.isPrivateTab = frame.get('isPrivate')
props.notificationBarActive = notificationBarActive

// used in renderer
props.frameKey = frameKey
props.isPinnedTab = isPinned
props.isPrivateTab = privateState.isPrivateTab(currentWindow, frameKey)
props.isActive = frameStateUtil.isFrameKeyActive(currentWindow, props.frameKey)
props.tabWidth = currentWindow.getIn(['ui', 'tabs', 'fixTabWidth'])
props.isPinnedTab = tabState.isTabPinned(state, tabId)
props.canPlayAudio = audioState.canPlayAudio(currentWindow, props.frameKey)
props.themeColor = tabUIState.getThemeColor(currentWindow, props.frameKey)
props.themeColor = tabUIState.getThemeColor(currentWindow, frameKey)
props.title = frame.get('title')
props.partOfFullPageSet = ownProps.partOfFullPageSet
props.showAudioTopBorder = audioState.showAudioTopBorder(currentWindow, frameKey, isPinned)
props.centralizeTabIcons = tabUIState.centralizeTabIcons(currentWindow, frameKey, isPinned)

// used in other functions
props.totalTabs = state.get('tabs').size
Expand All @@ -265,7 +276,7 @@ class Tab extends React.Component {
render () {
// we don't want themeColor if tab is private
const perPageStyles = !this.props.isPrivateTab && StyleSheet.create({
themeColor: {
tab_themeColor: {
color: this.props.themeColor ? getTextColorForBackground(this.props.themeColor) : 'inherit',
background: this.props.themeColor ? this.props.themeColor : 'inherit',
':hover': {
Expand Down Expand Up @@ -301,13 +312,15 @@ class Tab extends React.Component {
className={css(
styles.tab,
// Windows specific style
isWindows && styles.tabForWindows,
this.props.isPinnedTab && styles.isPinned,
this.props.isActive && styles.active,
this.props.isActive && this.props.themeColor && perPageStyles.themeColor,
isWindows && styles.tab_forWindows,
this.props.isPinnedTab && styles.tab_pinned,
this.props.isActive && styles.tab_active,
this.props.showAudioTopBorder && styles.tab_audioTopBorder,
this.props.isActive && this.props.themeColor && perPageStyles.tab_themeColor,
// Private color should override themeColor
this.props.isPrivateTab && styles.private,
this.props.isActive && this.props.isPrivateTab && styles.activePrivateTab
this.props.isPrivateTab && styles.tab_private,
this.props.isActive && this.props.isPrivateTab && styles.tab_active_private,
this.props.centralizeTabIcons && styles.tab__content_centered
)}
data-test-id='tab'
data-frame-key={this.props.frameKey}
Expand All @@ -323,7 +336,10 @@ class Tab extends React.Component {
ref={(node) => { this.tabSentinel = node }}
className={css(styles.tab__sentinel)}
/>
<div className={css(styles.tabId)}>
<div className={css(
styles.tab__identity,
this.props.centralizeTabIcons && styles.tab__content_centered
)}>
<Favicon tabId={this.props.tabId} />
<AudioTabIcon tabId={this.props.tabId} />
<TabTitle tabId={this.props.tabId} />
Expand All @@ -337,34 +353,39 @@ class Tab extends React.Component {
}

const styles = StyleSheet.create({
// Windows specific style
tabForWindows: {
color: '#555'
},

tab: {
borderWidth: '0 1px 0 0',
borderStyle: 'solid',
borderColor: '#bbb',
boxSizing: 'border-box',
color: '#5a5a5a',
color: theme.tab.color,
display: 'flex',
marginTop: '0',
transition: `transform 200ms ease, ${globalStyles.transition.tabBackgroundTransition}`,
transition: theme.tab.transition,
left: '0',
opacity: '1',
width: '100%',
height: '-webkit-fill-available',
width: '-webkit-fill-available',
alignItems: 'center',
justifyContent: 'space-between',
padding: globalStyles.spacing.defaultTabPadding,
padding: 0,
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: theme.tab.hover.background
}
},

// Windows specific style
tab_forWindows: {
color: theme.tab.forWindows.color
},

tab_active: {
background: theme.tab.active.background,

':hover': {
background: 'linear-gradient(to bottom, rgba(255, 255, 255, 0.8), rgba(250, 250, 250, 0.4))'
background: theme.tab.active.background
}
},

Expand All @@ -380,59 +401,56 @@ const styles = StyleSheet.create({
width: globalStyles.spacing.sentinelSize
},

tabId: {
justifyContent: 'center',
tab_audioTopBorder: {
'::before': {
content: `''`,
display: 'block',
position: 'absolute',
top: 0,
left: 0,
right: 0,
height: '2px',
background: 'lightskyblue'
}
},

tab__identity: {
justifyContent: 'flex-start',
alignItems: 'center',
display: 'flex',
flex: '1',

// @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'
minWidth: '0', // @see https://bugzilla.mozilla.org/show_bug.cgi?id=1108514#c5
marginLeft: globalStyles.spacing.defaultTabMargin
},

isPinned: {
tab__content_centered: {
justifyContent: 'center',
flex: 'auto',
padding: 0,
width: `calc(${globalStyles.spacing.tabsToolbarHeight} * 1.1)`,
justifyContent: 'center'
margin: 0
},

active: {
background: `rgba(255, 255, 255, 1.0)`,
marginTop: '0',
borderWidth: '0 1px 0 0',
borderStyle: 'solid',
borderColor: '#bbb',
color: '#000',

':hover': {
background: `linear-gradient(to bottom, #fff, ${globalStyles.color.chromePrimary})`
}
tab_pinned: {
padding: 0,
width: '28px',
justifyContent: 'center'
},

activePrivateTab: {
background: globalStyles.color.privateTabBackgroundActive,
tab_active_private: {
background: theme.tab.active.private.background,
color: theme.tab.active.private.color,

':hover': {
background: globalStyles.color.privateTabBackgroundActive
background: theme.tab.active.private.background
}
},

private: {
background: 'rgba(75, 60, 110, 0.2)',
tab_private: {
background: theme.tab.private.background,

':hover': {
background: globalStyles.color.privateTabBackgroundActive
}
},

dragging: {
':hover': {
closeTab: {
opacity: '0'
}
color: theme.tab.active.private.color,
background: theme.tab.active.private.background
}
}
})
Expand Down
12 changes: 9 additions & 3 deletions less/tabs.less
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
flex: 1;
overflow: auto;
padding: 0;
height: @tabsToolbarHeight;
height: -webkit-fill-available;
position: relative;
white-space: nowrap;
z-index: @zindexTabs;
Expand Down Expand Up @@ -59,6 +59,7 @@
position: relative;
vertical-align: top;
overflow: hidden;
height: -webkit-fill-available;
// There's a special case that tabs should span the full width
// if there are a full set of them.
&:not(.partOfFullPageSet) {
Expand Down Expand Up @@ -123,14 +124,17 @@
box-sizing: border-box;
background: @tabsBackground;
display: flex;
height: @tabsToolbarHeight;
// This element is set as border-box so it does not
// take into account the borders as width gutter, so we
// increase its size by 1px to include the top border
height: calc(~'@{tabsToolbarHeight} + 1px');
border-top: 1px solid @tabsToolbarBorderColor;
user-select: none;
}

.tabsToolbarButtons {
box-sizing: border-box;
height: @tabHeight;
height: -webkit-fill-available;
padding-right: 5px;

.browserButton {
Expand Down Expand Up @@ -222,6 +226,8 @@
}

.pinnedTabs {
height: -webkit-fill-available;
box-sizing: border-box;
margin-left: 0;
margin-top: 0;
background: @tabsBackgroundInactive;
Expand Down
2 changes: 1 addition & 1 deletion less/window.less
Original file line number Diff line number Diff line change
Expand Up @@ -174,5 +174,5 @@ body {
background-color: #000;
}
#windowContainer {
color: #f00;
color: #000;
}

1 comment on commit 23fcc08

@luixxiul
Copy link
Contributor

Choose a reason for hiding this comment

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

++ if comments on #10691 are addressed

Please sign in to comment.