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

Merges tabs into frames object in windowStore #8833

Merged
merged 1 commit into from
May 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions app/renderer/components/tabs/content/audioTabIcon.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@ const tabStyles = require('../../styles/tab')

class AudioTabIcon extends ImmutableComponent {
get pageCanPlayAudio () {
return !!this.props.tab.get('audioPlaybackActive')
return !!this.props.frame.get('audioPlaybackActive')
}

get shouldShowAudioIcon () {
// We switch to blue top bar for all breakpoints but default
return this.props.tab.get('breakpoint') === 'default'
return this.props.frame.get('breakpoint') === 'default'
}

get mutedState () {
return this.pageCanPlayAudio && !!this.props.tab.get('audioMuted')
return this.pageCanPlayAudio && !!this.props.frame.get('audioMuted')
}

get audioIcon () {
Expand All @@ -37,7 +37,8 @@ class AudioTabIcon extends ImmutableComponent {
return this.pageCanPlayAudio && this.shouldShowAudioIcon
? <TabIcon
className={css(tabStyles.icon, styles.audioIcon)}
symbol={this.audioIcon} onClick={this.props.onClick} />
symbol={this.audioIcon}
onClick={this.props.onClick} />
: null
}
}
Expand Down
2 changes: 1 addition & 1 deletion app/renderer/components/tabs/content/closeTabIcon.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const closeTabSvg = require('../../../../extensions/brave/img/tabs/close_btn_nor

class CloseTabIcon extends ImmutableComponent {
get isPinned () {
return !!this.props.tab.get('pinnedLocation')
return !!this.props.frame.get('pinnedLocation')
}

render () {
Expand Down
8 changes: 4 additions & 4 deletions app/renderer/components/tabs/content/favIcon.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const loadingIconSvg = require('../../../../extensions/brave/img/tabs/loading.sv

class Favicon extends ImmutableComponent {
get favicon () {
return !this.props.isLoading && this.props.tab.get('icon')
return !this.props.isLoading && this.props.frame.get('icon')
}

get defaultIcon () {
Expand All @@ -30,12 +30,12 @@ class Favicon extends ImmutableComponent {
}

get narrowView () {
return this.props.tab.get('breakpoint') === 'smallest'
return this.props.frame.get('breakpoint') === 'smallest'
}

get shouldHideFavicon () {
return (hasBreakpoint(this.props, 'extraSmall') && this.props.isActive) ||
this.props.tab.get('location') === 'about:newtab'
this.props.frame.get('location') === 'about:newtab'
}

render () {
Expand All @@ -57,7 +57,7 @@ class Favicon extends ImmutableComponent {
className={css(
tabStyles.icon,
this.favicon && iconStyles.favicon,
!this.props.tab.get('pinnedLocation') && this.narrowView && styles.faviconNarrowView
!this.props.frame.get('pinnedLocation') && this.narrowView && styles.faviconNarrowView
)}
symbol={
(this.props.isLoading && css(styles.loadingIcon, iconStyles.loadingIconColor)) ||
Expand Down
2 changes: 1 addition & 1 deletion app/renderer/components/tabs/content/newSessionIcon.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const newSessionSvg = require('../../../../extensions/brave/img/tabs/new_session

class NewSessionIcon extends ImmutableComponent {
get partitionNumber () {
let partition = this.props.tab.get('partitionNumber')
let partition = this.props.frame.get('partitionNumber')
// Persistent partitions opened by `target="_blank"` will have
// *partition-* string first, which causes bad UI. We don't need it for tabs
if (typeof partition === 'string') {
Expand Down
2 changes: 1 addition & 1 deletion app/renderer/components/tabs/content/privateIcon.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class PrivateIcon extends ImmutableComponent {
}
})

return this.props.tab.get('isPrivate') && hasVisibleSecondaryIcon(this.props)
return this.props.frame.get('isPrivate') && hasVisibleSecondaryIcon(this.props)
? <TabIcon data-test-id='privateIcon'
className={css(tabStyles.icon, styles.secondaryIcon, privateStyles.icon)}
/>
Expand Down
4 changes: 2 additions & 2 deletions app/renderer/components/tabs/content/tabTitle.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ const globalStyles = require('../../styles/global')
class TabTitle extends ImmutableComponent {
get isActiveOrHasSecondaryIcon () {
return this.props.isActive ||
(!!this.props.tab.get('isPrivate') || !!this.props.tab.get('partitionNumber'))
(!!this.props.frame.get('isPrivate') || !!this.props.frame.get('partitionNumber'))
}

get isPinned () {
return !!this.props.tab.get('pinnedLocation')
return !!this.props.frame.get('pinnedLocation')
}

get shouldHideTitle () {
Expand Down
12 changes: 6 additions & 6 deletions app/renderer/components/tabs/pinnedTabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ class PinnedTabs extends ImmutableComponent {
// will cause the onDragEnd to never run
setTimeout(() => {
const key = sourceDragData.get('key')
let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((node) => node && node.props.tab.get('frameKey') !== key), clientX).selectedRef
let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((node) => node && node.props.frame.get('key') !== key), clientX).selectedRef
if (droppedOnTab) {
const isLeftSide = dnd.isLeftSide(ReactDOM.findDOMNode(droppedOnTab), clientX)
const droppedOnFrameProps = windowStore.getFrame(droppedOnTab.props.tab.get('frameKey'))
const droppedOnFrameProps = windowStore.getFrame(droppedOnTab.props.frame.get('key'))
windowActions.moveTab(sourceDragData, droppedOnFrameProps, isLeftSide)
if (!sourceDragData.get('pinnedLocation')) {
appActions.tabPinned(sourceDragData.get('tabId'), true)
Expand All @@ -73,14 +73,14 @@ class PinnedTabs extends ImmutableComponent {
onDrop={this.onDrop}>
{
this.props.pinnedTabs
.map((tab) =>
.map((frame) =>
<Tab ref={(node) => this.tabRefs.push(node)}
dragData={this.props.dragData}
tab={tab}
key={'tab-' + tab.get('frameKey')}
frame={frame}
key={'tab-' + frame.get('key')}
paintTabs={this.props.paintTabs}
previewTabs={this.props.previewTabs}
isActive={this.props.activeFrameKey === tab.get('frameKey')}
isActive={this.props.activeFrameKey === frame.get('key')}
partOfFullPageSet={this.props.partOfFullPageSet} />)
}
</div>
Expand Down
78 changes: 39 additions & 39 deletions app/renderer/components/tabs/tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,16 @@ class Tab extends ImmutableComponent {
this.onUpdateTabSize = this.onUpdateTabSize.bind(this)
}
get frame () {
return windowStore.getFrame(this.props.tab.get('frameKey'))
return windowStore.getFrame(this.props.frame.get('key'))
}
get isPinned () {
return !!this.props.tab.get('pinnedLocation')
return !!this.props.frame.get('pinnedLocation')
}

get draggingOverData () {
const draggingOverData = this.props.dragData && this.props.dragData.get('dragOverData')
if (!draggingOverData ||
draggingOverData.get('draggingOverKey') !== this.props.tab.get('frameKey') ||
draggingOverData.get('draggingOverKey') !== this.props.frame.get('key') ||
draggingOverData.get('draggingOverWindowId') !== getCurrentWindowId()) {
return
}
Expand All @@ -83,7 +83,7 @@ class Tab extends ImmutableComponent {
get isDragging () {
const sourceDragData = dnd.getInterBraveDragData()
return sourceDragData &&
sourceDragData.get('key') === this.props.tab.get('frameKey') &&
sourceDragData.get('key') === this.props.frame.get('key') &&
sourceDragData.get('draggingOverWindowId') === getCurrentWindowId()
}

Expand Down Expand Up @@ -115,17 +115,17 @@ class Tab extends ImmutableComponent {
// until we know what we're loading. We should probably do this for
// all about: pages that we already know the title for so we don't have
// to wait for the title to be parsed.
if (this.props.tab.get('location') === 'about:blank') {
if (this.props.frame.get('location') === 'about:blank') {
return locale.translation('aboutBlankTitle')
} else if (this.props.tab.get('location') === 'about:newtab') {
} else if (this.props.frame.get('location') === 'about:newtab') {
return locale.translation('newTab')
}

// YouTube tries to change the title to add a play icon when
// there is audio. Since we have our own audio indicator we get
// rid of it.
return (this.props.tab.get('title') ||
this.props.tab.get('location') ||
return (this.props.frame.get('title') ||
this.props.frame.get('location') ||
'').replace('▶ ', '')
}

Expand All @@ -138,7 +138,7 @@ class Tab extends ImmutableComponent {
}

onDragOver (e) {
dnd.onDragOver(dragTypes.TAB, this.tabNode.getBoundingClientRect(), this.props.tab.get('frameKey'), this.draggingOverData, e)
dnd.onDragOver(dragTypes.TAB, this.tabNode.getBoundingClientRect(), this.props.frame.get('key'), this.draggingOverData, e)
}

onTabClosedWithMouse (event) {
Expand All @@ -157,10 +157,10 @@ class Tab extends ImmutableComponent {

get loading () {
return this.frame &&
(this.props.tab.get('loading') ||
this.props.tab.get('location') === 'about:blank') &&
(!this.props.tab.get('provisionalLocation') ||
!this.props.tab.get('provisionalLocation').startsWith('chrome-extension://mnojpmjdmbbfmejpflffifhffcmidifd/'))
(this.props.frame.get('loading') ||
this.props.frame.get('location') === 'about:blank') &&
(!this.props.frame.get('provisionalLocation') ||
!this.props.frame.get('provisionalLocation').startsWith('chrome-extension://mnojpmjdmbbfmejpflffifhffcmidifd/'))
}

onMouseLeave () {
Expand Down Expand Up @@ -201,7 +201,7 @@ class Tab extends ImmutableComponent {

get themeColor () {
return this.props.paintTabs &&
(this.props.tab.get('themeColor') || this.props.tab.get('computedThemeColor'))
(this.props.frame.get('themeColor') || this.props.frame.get('computedThemeColor'))
}

get tabSize () {
Expand All @@ -212,21 +212,21 @@ class Tab extends ImmutableComponent {

get mediumView () {
const sizes = ['large', 'largeMedium']
return sizes.includes(this.props.tab.get('breakpoint'))
return sizes.includes(this.props.frame.get('breakpoint'))
}

get narrowView () {
const sizes = ['medium', 'mediumSmall', 'small', 'extraSmall', 'smallest']
return sizes.includes(this.props.tab.get('breakpoint'))
return sizes.includes(this.props.frame.get('breakpoint'))
}

get narrowestView () {
const sizes = ['extraSmall', 'smallest']
return sizes.includes(this.props.tab.get('breakpoint'))
return sizes.includes(this.props.frame.get('breakpoint'))
}

get canPlayAudio () {
return this.props.tab.get('audioPlaybackActive') || this.props.tab.get('audioMuted')
return this.props.frame.get('audioPlaybackActive') || this.props.frame.get('audioMuted')
}

onUpdateTabSize () {
Expand Down Expand Up @@ -263,10 +263,10 @@ class Tab extends ImmutableComponent {

render () {
const notificationBarActive = !!this.props.notificationBarActive &&
this.props.notificationBarActive.includes(UrlUtil.getUrlOrigin(this.props.tab.get('location')))
this.props.notificationBarActive.includes(UrlUtil.getUrlOrigin(this.props.frame.get('location')))
const playIndicatorBreakpoint = this.mediumView || this.narrowView
// we don't want themeColor if tab is private
const perPageStyles = !this.props.tab.get('isPrivate') && StyleSheet.create({
const perPageStyles = !this.props.frame.get('isPrivate') && StyleSheet.create({
themeColor: {
color: this.themeColor ? getTextColorForBackground(this.themeColor) : 'inherit',
background: this.themeColor ? this.themeColor : 'inherit',
Expand Down Expand Up @@ -302,20 +302,20 @@ class Tab extends ImmutableComponent {
playIndicatorBreakpoint && this.canPlayAudio && styles.narrowViewPlayIndicator,
this.props.isActive && this.themeColor && perPageStyles.themeColor,
// Private color should override themeColor
this.props.tab.get('isPrivate') && styles.private,
this.props.isActive && this.props.tab.get('isPrivate') && styles.activePrivateTab,
this.props.frame.get('isPrivate') && styles.private,
this.props.isActive && this.props.frame.get('isPrivate') && styles.activePrivateTab,
!this.isPinned && this.narrowView && styles.tabNarrowView,
!this.isPinned && this.narrowestView && styles.tabNarrowestView,
!this.isPinned && this.props.tab.get('breakpoint') === 'smallest' && styles.tabMinAllowedSize
!this.isPinned && this.props.frame.get('breakpoint') === 'smallest' && styles.tabMinAllowedSize
)}
data-test-active-tab={this.props.isActive}
data-test-pinned-tab={this.isPinned}
data-test-private-tab={this.props.tab.get('isPrivate')}
data-test-private-tab={this.props.frame.get('isPrivate')}
data-test-id='tab'
data-frame-key={this.props.tab.get('frameKey')}
data-frame-key={this.props.frame.get('key')}
ref={(node) => { this.tabNode = node }}
draggable
title={this.props.tab.get('title')}
title={this.props.frame.get('title')}
onDragStart={this.onDragStart.bind(this)}
onDragEnd={this.onDragEnd.bind(this)}
onDragOver={this.onDragOver.bind(this)}
Expand All @@ -324,40 +324,40 @@ class Tab extends ImmutableComponent {
<div className={css(
styles.tabId,
this.narrowView && styles.tabIdNarrowView,
this.props.tab.get('breakpoint') === 'smallest' && styles.tabIdMinAllowedSize
this.props.frame.get('breakpoint') === 'smallest' && styles.tabIdMinAllowedSize
)}>
<Favicon
isActive={this.props.isActive}
paintTabs={this.props.paintTabs}
tab={this.props.tab}
frame={this.props.frame}
Copy link
Member

Choose a reason for hiding this comment

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

It might be better to create a reference for this (ex: const frame = this.props.frame) and then just pass the frame={frame}, instead of this.props.frame each time. This way, if it changes again, you only change one place

Copy link
Member

Choose a reason for hiding this comment

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

Same with all the stuff above; getting rid of all the duplicate this.props.

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 will remove all this props in redux PR, so I think it's not necessary to do it right now.

Copy link
Member

Choose a reason for hiding this comment

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

SGTM 👍 😄

isLoading={this.loading}
isPinned={this.isPinned}
/>
<AudioTabIcon
tab={this.props.tab}
onClick={this.onMuteFrame.bind(this, !this.props.tab.get('audioMuted'))}
frame={this.props.frame}
onClick={this.onMuteFrame.bind(this, !this.props.frame.get('audioMuted'))}
/>
<TabTitle
isActive={this.props.isActive}
paintTabs={this.props.paintTabs}
tab={this.props.tab}
frame={this.props.frame}
pageTitle={this.displayValue}
/>
</div>
<PrivateIcon
tab={this.props.tab}
frame={this.props.frame}
isActive={this.props.isActive}
/>
<NewSessionIcon
isActive={this.props.isActive}
paintTabs={this.props.paintTabs}
tab={this.props.tab}
l10nArgs={this.props.tab.get('partitionNumber')}
frame={this.props.frame}
l10nArgs={this.props.frame.get('partitionNumber')}
l10nId='sessionInfoTab'
/>
<CloseTabIcon
isActive={this.props.isActive}
tab={this.props.tab}
frame={this.props.frame}
onClick={this.onTabClosedWithMouse.bind(this)}
l10nId='closeTabButton'
/>
Expand All @@ -375,11 +375,11 @@ const paymentsEnabled = () => {
windowStore.addChangeListener(() => {
if (paymentsEnabled()) {
const windowState = windowStore.getState()
const tabs = windowState && windowState.get('tabs')
if (tabs) {
const frames = windowState && windowState.get('frames')
if (frames) {
try {
const presentP = tabs.some((tab) => {
return tab.get('location') === 'about:preferences#payments'
const presentP = frames.some((frame) => {
return frame.get('location') === 'about:preferences#payments'
})
ipc.send(messages.LEDGER_PAYMENTS_PRESENT, presentP)
} catch (ex) { }
Expand Down
12 changes: 6 additions & 6 deletions app/renderer/components/tabs/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ class Tabs extends ImmutableComponent {
// will cause the onDragEnd to never run
setTimeout(() => {
const key = sourceDragData.get('key')
let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((node) => node && node.props.tab.get('frameKey') !== key), clientX).selectedRef
let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((node) => node && node.props.frame.get('key') !== key), clientX).selectedRef
if (droppedOnTab) {
const isLeftSide = dnd.isLeftSide(ReactDOM.findDOMNode(droppedOnTab), clientX)
const droppedOnFrameProps = windowStore.getFrame(droppedOnTab.props.tab.get('frameKey'))
const droppedOnFrameProps = windowStore.getFrame(droppedOnTab.props.frame.get('key'))

// If this is a different window ID than where the drag started, then
// the tear off will be done by tab.js
Expand Down Expand Up @@ -149,14 +149,14 @@ class Tabs extends ImmutableComponent {
})()}
{
this.props.currentTabs
.map((tab) =>
.map((frame) =>
<Tab ref={(node) => this.tabRefs.push(node)}
dragData={this.props.dragData}
tab={tab}
key={'tab-' + tab.get('frameKey')}
frame={frame}
key={'tab-' + frame.get('key')}
paintTabs={this.props.paintTabs}
previewTabs={this.props.previewTabs}
isActive={this.props.activeFrameKey === tab.get('frameKey')}
isActive={this.props.activeFrameKey === frame.get('key')}
onTabClosedWithMouse={this.onTabClosedWithMouse}
tabWidth={this.props.fixTabWidth}
hasTabInFullScreen={this.props.hasTabInFullScreen}
Expand Down
4 changes: 2 additions & 2 deletions app/renderer/components/tabs/tabsToolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ class TabsToolbar extends ImmutableComponent {
const index = this.props.previewTabPageIndex !== undefined
? this.props.previewTabPageIndex : this.props.tabPageIndex
const startingFrameIndex = index * this.props.tabsPerTabPage
const pinnedTabs = this.props.tabs.filter((tab) => tab.get('pinnedLocation'))
const unpinnedTabs = this.props.tabs.filter((tab) => !tab.get('pinnedLocation'))
const pinnedTabs = this.props.frames.filter((tab) => tab.get('pinnedLocation'))
const unpinnedTabs = this.props.frames.filter((tab) => !tab.get('pinnedLocation'))
const currentTabs = unpinnedTabs
.slice(startingFrameIndex, startingFrameIndex + this.props.tabsPerTabPage)
return <div className='tabsToolbar'
Expand Down
Loading