Skip to content

Commit 189bb1b

Browse files
manavbpjoshuef
manavbp
authored and
joshuef
committed
Fix(Menu): Scope open menu to Current Window only
1 parent 94e8b43 commit 189bb1b

11 files changed

Lines changed: 226 additions & 72 deletions

File tree

‎__tests__/actions/ui.spec.js‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,13 @@ describe( 'ui actions', () =>
66
{
77
expect( ui.TYPES ).toBeDefined();
88
} );
9-
9+
it( 'should add window to UI store', () =>
10+
{
11+
const expectedAction = {
12+
type : ui.TYPES.UI_ADD_WINDOW
13+
};
14+
expect( ui.uiAddWindow() ).toEqual( expectedAction );
15+
} );
1016
it( 'should show settings menu', () =>
1117
{
1218
const expectedAction = {
@@ -22,7 +28,13 @@ describe( 'ui actions', () =>
2228
};
2329
expect( ui.hideSettingsMenu() ).toEqual( expectedAction );
2430
} );
25-
31+
it( 'should remove window from UI store', () =>
32+
{
33+
const expectedAction = {
34+
type : ui.TYPES.UI_REMOVE_WINDOW
35+
};
36+
expect( ui.uiRemoveWindow() ).toEqual( expectedAction );
37+
} );
2638
it( 'should set addressbar selected', () =>
2739
{
2840
const expectedAction = {

‎__tests__/components/Browser.spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jest.autoMockOff();
1414

1515
// create any initial state needed
1616
const initialState = {
17-
ui : {},
17+
ui : { windows: [] },
1818
tabs : [],
1919
windowId : 1,
2020
addTab : jest.fn(),

‎__tests__/reducers/ui.spec.js‎

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,47 @@ import initialState from 'reducers/initialAppState';
55

66
describe( 'notification reducer', () =>
77
{
8+
const uiInitialState = {
9+
windows : []
10+
};
11+
const uiStateShow = {
12+
windows : [ { windowId: 1, settingsMenuIsVisible: true } ]
13+
};
14+
const uiStateHide = {
15+
windows : [ { windowId: 1, settingsMenuIsVisible: false } ]
16+
};
817
it( 'should return the initial state', () =>
918
{
1019
expect( ui( undefined, {} ) ).toEqual( initialState.ui );
1120
} );
12-
21+
describe( 'UI_ADD_WINDOW', () =>
22+
{
23+
it( 'should handle add window to ui store', () =>
24+
{
25+
expect(
26+
ui(
27+
{ windows: [] },
28+
{
29+
type : TYPES.UI_ADD_WINDOW,
30+
payload : { windowId: 1 }
31+
}
32+
)
33+
).toEqual( uiStateHide );
34+
} );
35+
} );
1336
describe( 'SHOW_SETTINGS_MENU', () =>
1437
{
1538
it( 'should handle showing the settings menu', () =>
1639
{
1740
expect(
1841
ui(
19-
{},
42+
{ windows: [ { windowId: 1, settingsMenuIsVisible: false } ] },
2043
{
21-
type : TYPES.SHOW_SETTINGS_MENU
44+
type : TYPES.SHOW_SETTINGS_MENU,
45+
payload : { windowId: 1 }
2246
}
2347
)
24-
).toEqual( { settingsMenuIsVisible: true } );
48+
).toEqual( uiStateShow );
2549
} );
2650
} );
2751

@@ -31,15 +55,30 @@ describe( 'notification reducer', () =>
3155
{
3256
expect(
3357
ui(
34-
{},
58+
{ windows: [ { windowId: 1, settingsMenuIsVisible: true } ] },
3559
{
36-
type : TYPES.HIDE_SETTINGS_MENU
60+
type : TYPES.HIDE_SETTINGS_MENU,
61+
payload : { windowId: 1 }
3762
}
3863
)
39-
).toEqual( { settingsMenuIsVisible: false } );
64+
).toEqual( uiStateHide );
65+
} );
66+
} );
67+
describe( 'UI_REMOVE_WINDOW', () =>
68+
{
69+
it( 'should handle remove window from ui store', () =>
70+
{
71+
expect(
72+
ui(
73+
{ windows: [ { windowId: 1, settingsMenuIsVisible: false } ] },
74+
{
75+
type : TYPES.UI_REMOVE_WINDOW,
76+
payload : { windowId: 1 }
77+
}
78+
)
79+
).toEqual(uiInitialState);
4080
} );
4181
} );
42-
4382
describe( 'SELECT_ADDRESS_BAR', () =>
4483
{
4584
it( 'should handle setting address bar focus', () =>

‎app/actions/ui_actions.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ export const TYPES = {
44
SHOW_SETTINGS_MENU : 'SHOW_SETTINGS_MENU',
55
HIDE_SETTINGS_MENU : 'HIDE_SETTINGS_MENU',
66
BLUR_ADDRESS_BAR : 'BLUR_ADDRESS_BAR',
7+
UI_ADD_WINDOW : 'UI_ADD_WINDOW',
8+
UI_REMOVE_WINDOW : 'UI_REMOVE_WINDOW',
79
DESELECT_ADDRESS_BAR : 'DESELECT_ADDRESS_BAR',
810
SELECT_ADDRESS_BAR : 'SELECT_ADDRESS_BAR',
911
RESET_STORE : 'RESET_STORE',
@@ -15,15 +17,19 @@ export const {
1517
hideSettingsMenu,
1618
blurAddressBar,
1719
deselectAddressBar,
20+
uiAddWindow,
1821
selectAddressBar,
22+
uiRemoveWindow,
1923
resetStore,
2024
focusWebview
2125
} = createActions(
2226
TYPES.SHOW_SETTINGS_MENU,
2327
TYPES.HIDE_SETTINGS_MENU,
2428
TYPES.BLUR_ADDRESS_BAR,
2529
TYPES.DESELECT_ADDRESS_BAR,
30+
TYPES.UI_ADD_WINDOW,
2631
TYPES.SELECT_ADDRESS_BAR,
32+
TYPES.UI_REMOVE_WINDOW,
2733
TYPES.RESET_STORE,
2834
TYPES.FOCUS_WEBVIEW
2935
);

‎app/components/AddressBar/AddressBar.jsx‎

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -73,48 +73,49 @@ export default class AddressBar extends Component
7373
const { windowId } = this.props;
7474
const addATab = tab =>
7575
{
76-
addTab( { url: `safe-browser://${tab}`, isActiveTab: true, windowId } );
76+
addTab( { url: `safe-browser://${ tab }`, isActiveTab: true, windowId } );
7777
};
7878

7979
return [
8080
<Row
81-
key={'menuItem-bookmarks'}
81+
key="menuItem-bookmarks"
8282
type="flex"
8383
justify="start"
8484
align="middle"
8585
>
8686
<div
8787
role="menuitem"
88-
tabIndex={0}
89-
className={`${styles.menuItem} ${
88+
tabIndex={ 0 }
89+
className={ `${ styles.menuItem } ${
9090
CLASSES.SETTINGS_MENU__BOOKMARKS
91-
}`}
92-
onClick={() => addATab('bookmarks')}
91+
}` }
92+
onClick={ () => addATab( 'bookmarks' ) }
9393
>
9494
Bookmarks
9595
</div>
9696
</Row>,
9797
<Row
98-
key={'menuItem-history'}
98+
key="menuItem-history"
9999
type="flex"
100100
justify="start"
101101
align="middle"
102102
>
103103
<div
104104
role="menuitem"
105-
tabIndex={0}
106-
className={`${styles.menuItem} ${
105+
tabIndex={ 0 }
106+
className={ `${ styles.menuItem } ${
107107
CLASSES.SETTINGS_MENU__HISTORY
108-
}`}
109-
onClick={() => addATab('history')}
108+
}` }
109+
onClick={ () => addATab( 'history' ) }
110110
>
111111
History
112112
</div>
113113
</Row>
114114
];
115115
};
116116

117-
render() {
117+
render()
118+
{
118119
const props = this.props;
119120

120121
const {
@@ -124,6 +125,7 @@ export default class AddressBar extends Component
124125
removeBookmark,
125126
isBookmarked,
126127
activeTab,
128+
windowId,
127129
updateTab,
128130
settingsMenuIsVisible,
129131
showSettingsMenu,
@@ -137,13 +139,13 @@ export default class AddressBar extends Component
137139
: false;
138140

139141
return (
140-
<div className={`${styles.container} js-address`}>
142+
<div className={ `${ styles.container } js-address` }>
141143
<Row
142-
className={styles.addressBar}
144+
className={ styles.addressBar }
143145
type="flex"
144146
justify="start"
145147
align="middle"
146-
gutter={{ xs: 4, sm: 8, md: 12 }}
148+
gutter={ { xs: 4, sm: 8, md: 12 } }
147149
>
148150
<Col>
149151
<ButtonsLHS
@@ -157,21 +159,22 @@ export default class AddressBar extends Component
157159
{ ...props }
158160
/>
159161
</Col>
160-
<Col className={styles.addressBarCol}>
161-
<Input {...this.props} />
162+
<Col className={ styles.addressBarCol }>
163+
<Input { ...this.props } />
162164
</Col>
163165
<Col>
164166
<ButtonsRHS
165-
address={address}
166-
addTab={addTab}
167-
isBookmarked={isBookmarked}
168-
addBookmark={addBookmark}
169-
removeBookmark={removeBookmark}
170-
menuItems={this.getSettingsMenuItems()}
171-
showSettingsMenu={showSettingsMenu}
172-
settingsMenuIsVisible={settingsMenuIsVisible}
173-
hideSettingsMenu={hideSettingsMenu}
174-
focusWebview={focusWebview}
167+
address={ address }
168+
addTab={ addTab }
169+
isBookmarked={ isBookmarked }
170+
addBookmark={ addBookmark }
171+
windowId={ windowId }
172+
removeBookmark={ removeBookmark }
173+
menuItems={ this.getSettingsMenuItems() }
174+
showSettingsMenu={ showSettingsMenu }
175+
settingsMenuIsVisible={ settingsMenuIsVisible }
176+
hideSettingsMenu={ hideSettingsMenu }
177+
focusWebview={ focusWebview }
175178
/>
176179
</Col>
177180
</Row>

‎app/components/AddressBar/ButtonsRHS/ButtonsRHS.jsx‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,11 @@ class ButtonsRHS extends Component {
2626
address: PropTypes.string,
2727
isBookmarked: PropTypes.bool.isRequired,
2828
addBookmark: PropTypes.func.isRequired,
29-
removeBookmark: PropTypes.func.isRequired,
30-
showSettingsMenu: PropTypes.func.isRequired,
31-
hideSettingsMenu: PropTypes.func.isRequired,
32-
settingsMenuIsVisible: PropTypes.bool.isRequired,
29+
removeBookmark : PropTypes.func.isRequired,
30+
showSettingsMenu : PropTypes.func.isRequired,
31+
windowId : PropTypes.number.isRequired,
32+
hideSettingsMenu : PropTypes.func.isRequired,
33+
settingsMenuIsVisible : PropTypes.bool.isRequired,
3334
menuItems: PropTypes.arrayOf(PropTypes.node).isRequired,
3435
focusWebview: PropTypes.func.isRequired
3536
};
@@ -60,6 +61,7 @@ class ButtonsRHS extends Component {
6061
isBookmarked,
6162
settingsMenuIsVisible,
6263
addTab,
64+
windowId,
6365
showSettingsMenu,
6466
hideSettingsMenu,
6567
menuItems,
@@ -99,6 +101,7 @@ class ButtonsRHS extends Component {
99101
<Col>
100102
<CustomMenu
101103
isVisible={settingsMenuIsVisible}
104+
windowId={ windowId }
102105
menuItems={menuItems}
103106
showMenu={showSettingsMenu}
104107
hideMenu={hideSettingsMenu}

‎app/components/Browser/Browser.jsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,14 @@ class Browser extends Component
181181
}
182182

183183
const activeTabAddress = activeTab.url;
184-
185184
const isBookmarked = !!bookmarks.find(
186185
bookmark => bookmark.url === activeTabAddress
187186
);
188-
187+
const uiWindow = ui.windows;
188+
const windowObjForCurrentWindow = uiWindow.find( function ( element ) {
189+
return element.windowId === windowId;
190+
});
191+
const settingsMenuIsVisible = windowObjForCurrentWindow ? windowObjForCurrentWindow.settingsMenuIsVisible : false;
189192
return (
190193
<div className={ styles.container }>
191194
<TabBar
@@ -210,7 +213,7 @@ class Browser extends Component
210213
removeBookmark={ removeBookmark }
211214
hideSettingsMenu={ hideSettingsMenu }
212215
showSettingsMenu={ showSettingsMenu }
213-
settingsMenuIsVisible={ ui.settingsMenuIsVisible }
216+
settingsMenuIsVisible={ settingsMenuIsVisible }
214217
isSelected={ ui.addressBarIsSelected }
215218
tabBackwards={ tabBackwards }
216219
tabForwards={ tabForwards }

‎app/components/CustomMenu/CustomMenu.jsx‎

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -34,38 +34,43 @@ const Meatball = () => (
3434
*/
3535
export default class CustomMenu extends Component {
3636
static propTypes = {
37-
isVisible: PropTypes.bool,
38-
menuItems: PropTypes.array,
39-
showMenu: PropTypes.func.isRequired,
40-
hideMenu: PropTypes.func.isRequired
37+
isVisible : PropTypes.bool,
38+
menuItems : PropTypes.array,
39+
showMenu : PropTypes.func.isRequired,
40+
hideMenu : PropTypes.func.isRequired,
41+
windowId : PropTypes.number.isRequired,
4142
};
4243

4344
static defaultProps = {
44-
isVisible: false,
45-
menuItems: []
45+
isVisible : false,
46+
menuItems : []
4647
};
4748

48-
handleShowingMenu = event => {
49+
handleShowingMenu = event =>
50+
{
4951
event.nativeEvent.stopImmediatePropagation();
5052

51-
const { showMenu, hideMenu, isVisible } = this.props;
53+
const { showMenu, hideMenu, isVisible, windowId } = this.props;
5254

53-
if (isVisible) {
54-
hideMenu();
55-
} else {
56-
showMenu();
57-
58-
const windowClickListener = event => {
59-
hideMenu();
55+
if ( isVisible )
56+
{
57+
hideMenu( { windowId } );
58+
}
59+
else
60+
{
61+
showMenu( { windowId } );
62+
const windowClickListener = event =>
63+
{
64+
hideMenu( { windowId } );
6065
};
61-
6266
window.addEventListener('click', windowClickListener, {
6367
once: true
64-
});
68+
} );
6569
}
6670
};
6771

68-
render() {
72+
render()
73+
{
6974
const { isVisible, menuItems } = this.props;
7075

7176
return (

0 commit comments

Comments
 (0)