Skip to content

Commit 2b7596d

Browse files
dewanshDTdewanshDT
authored
Merge pull request #3 from lindapaiste/fix/mobilenav-accessibility
Use core Nav logic in MobileNav.
2 parents 99de115 + 0180a67 commit 2b7596d

File tree

9 files changed

+759
-348
lines changed

9 files changed

+759
-348
lines changed

‎client/components/Nav/NavBar.jsx

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import React, {
88
} from 'react';
99
import { MenuOpenContext, NavBarContext } from './contexts';
1010

11-
function NavBar({ children }) {
11+
function NavBar({ children, className }) {
1212
const [dropdownOpen, setDropdownOpen] = useState('none');
1313

1414
const timerRef = useRef(null);
@@ -55,6 +55,15 @@ function NavBar({ children }) {
5555
timerRef.current = setTimeout(() => setDropdownOpen('none'), 10);
5656
}, [timerRef, setDropdownOpen]);
5757

58+
const toggleDropdownOpen = useCallback(
59+
(dropdown) => {
60+
setDropdownOpen((prevState) =>
61+
prevState === dropdown ? 'none' : dropdown
62+
);
63+
},
64+
[setDropdownOpen]
65+
);
66+
5867
const contextValue = useMemo(
5968
() => ({
6069
createDropdownHandlers: (dropdown) => ({
@@ -64,9 +73,7 @@ function NavBar({ children }) {
6473
);
6574
},
6675
onClick: () => {
67-
setDropdownOpen((prevState) =>
68-
prevState === 'none' ? dropdown : 'none'
69-
);
76+
toggleDropdownOpen(dropdown);
7077
},
7178
onBlur: handleBlur,
7279
onFocus: clearHideTimeout
@@ -80,15 +87,16 @@ function NavBar({ children }) {
8087
clearHideTimeout();
8188
setDropdownOpen(dropdown);
8289
}
83-
})
90+
}),
91+
toggleDropdownOpen
8492
}),
85-
[setDropdownOpen, clearHideTimeout, handleBlur]
93+
[setDropdownOpen, toggleDropdownOpen, clearHideTimeout, handleBlur]
8694
);
8795

8896
return (
8997
<NavBarContext.Provider value={contextValue}>
9098
<header>
91-
<nav className="nav" ref={nodeRef}>
99+
<nav className={className} ref={nodeRef}>
92100
<MenuOpenContext.Provider value={dropdownOpen}>
93101
{children}
94102
</MenuOpenContext.Provider>
@@ -99,11 +107,13 @@ function NavBar({ children }) {
99107
}
100108

101109
NavBar.propTypes = {
102-
children: PropTypes.node
110+
children: PropTypes.node,
111+
className: PropTypes.string
103112
};
104113

105114
NavBar.defaultProps = {
106-
children: null
115+
children: null,
116+
className: 'nav'
107117
};
108118

109119
export default NavBar;

‎client/components/Nav/NavDropdownMenu.jsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import React, { useContext, useMemo } from 'react';
44
import TriangleIcon from '../../images/down-filled-triangle.svg';
55
import { MenuOpenContext, NavBarContext, ParentMenuContext } from './contexts';
66

7-
function NavDropdownMenu({ id, title, children }) {
7+
export function useMenuProps(id) {
88
const activeMenu = useContext(MenuOpenContext);
99

1010
const isOpen = id === activeMenu;
@@ -16,6 +16,12 @@ function NavDropdownMenu({ id, title, children }) {
1616
id
1717
]);
1818

19+
return { isOpen, handlers };
20+
}
21+
22+
function NavDropdownMenu({ id, title, children }) {
23+
const { isOpen, handlers } = useMenuProps(id);
24+
1925
return (
2026
<li className={classNames('nav__item', isOpen && 'nav__item--open')}>
2127
<button {...handlers}>

‎client/components/Nav/NavMenuItem.jsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import React, { useContext, useMemo } from 'react';
33
import ButtonOrLink from '../../common/ButtonOrLink';
44
import { NavBarContext, ParentMenuContext } from './contexts';
55

6-
function NavMenuItem({ hideIf, ...rest }) {
6+
function NavMenuItem({ hideIf, className, ...rest }) {
77
const parent = useContext(ParentMenuContext);
88

99
const { createMenuItemHandlers } = useContext(NavBarContext);
@@ -18,7 +18,7 @@ function NavMenuItem({ hideIf, ...rest }) {
1818
}
1919

2020
return (
21-
<li className="nav__dropdown-item">
21+
<li className={className}>
2222
<ButtonOrLink {...rest} {...handlers} />
2323
</li>
2424
);
@@ -31,13 +31,15 @@ NavMenuItem.propTypes = {
3131
/**
3232
* Provides a way to deal with optional items.
3333
*/
34-
hideIf: PropTypes.bool
34+
hideIf: PropTypes.bool,
35+
className: PropTypes.string
3536
};
3637

3738
NavMenuItem.defaultProps = {
3839
onClick: null,
3940
value: null,
40-
hideIf: false
41+
hideIf: false,
42+
className: 'nav__dropdown-item'
4143
};
4244

4345
export default NavMenuItem;

‎client/components/Nav/contexts.jsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,6 @@ export const MenuOpenContext = createContext('none');
66

77
export const NavBarContext = createContext({
88
createDropdownHandlers: () => ({}),
9-
createMenuItemHandlers: () => ({})
9+
createMenuItemHandlers: () => ({}),
10+
toggleDropdownOpen: () => {}
1011
});

‎client/modules/IDE/components/Header/MobileNav.jsx

Lines changed: 57 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@ import styled from 'styled-components';
33
import { useDispatch, useSelector } from 'react-redux';
44
import { useTranslation } from 'react-i18next';
55
import { useLocation } from 'react-router';
6-
import PropTypes from 'prop-types';
76
import { Link } from 'react-router-dom';
7+
import { ParentMenuContext } from '../../../../components/Nav/contexts';
8+
import NavBar from '../../../../components/Nav/NavBar';
9+
import { useMenuProps } from '../../../../components/Nav/NavDropdownMenu';
10+
import NavMenuItem from '../../../../components/Nav/NavMenuItem';
811
import { prop, remSize } from '../../../../theme';
912
import AsteriskIcon from '../../../../images/p5-asterisk.svg';
1013
import IconButton from '../../../../components/mobile/IconButton';
@@ -20,7 +23,7 @@ import { useSketchActions } from '../../hooks';
2023
import { CmControllerContext } from '../../pages/IDEView';
2124
import { selectSketchPath } from '../../selectors/project';
2225

23-
const Nav = styled.div`
26+
const Nav = styled(NavBar)`
2427
background: ${prop('MobilePanel.default.background')};
2528
color: ${prop('primaryTextColor')};
2629
padding: ${remSize(8)} 0;
@@ -74,8 +77,7 @@ const Options = styled.div`
7477
}
7578
7679
/* Drop Down menu */
77-
> div > button:focus + ul,
78-
> div > ul > button:focus ~ div > ul {
80+
ul.opened {
7981
transform: scale(1);
8082
opacity: 1;
8183
}
@@ -130,9 +132,7 @@ const Options = styled.div`
130132
> li {
131133
display: flex;
132134
width: 100%;
133-
a {
134-
width: 100%;
135-
}
135+
a,
136136
button {
137137
width: 100%;
138138
padding: ${remSize(8)} ${remSize(15)} ${remSize(8)} ${remSize(10)};
@@ -148,7 +148,7 @@ const Options = styled.div`
148148
}
149149
`;
150150

151-
const MobileNav = (props) => {
151+
const MobileNav = () => {
152152
const project = useSelector((state) => state.project);
153153
const user = useSelector((state) => state.user);
154154

@@ -222,24 +222,22 @@ const AccountMenu = () => {
222222
const user = useSelector((state) => state.user);
223223
const dispatch = useDispatch();
224224

225+
const { isOpen, handlers } = useMenuProps('account');
226+
225227
return (
226228
<div>
227-
<IconButton icon={AccountIcon} />
228-
<ul>
229-
<li className="user">{user.username}</li>
230-
<li>
231-
<Link to={`/${user.username}/sketches`}>
232-
<button>My Stuff</button>
233-
</Link>
234-
</li>
235-
<li>
236-
<Link to="/account">
237-
<button>Settings</button>
238-
</Link>
239-
</li>
240-
<li>
241-
<button onClick={() => dispatch(logoutUser())}>Log Out</button>
242-
</li>
229+
<IconButton icon={AccountIcon} {...handlers} />
230+
<ul className={isOpen ? 'opened' : ''}>
231+
<ParentMenuContext.Provider value="account">
232+
<li className="user">{user.username}</li>
233+
<NavMenuItem href={`/${user.username}/sketches`}>
234+
My Stuff
235+
</NavMenuItem>
236+
<NavMenuItem href="/account">Settings</NavMenuItem>
237+
<NavMenuItem onClick={() => dispatch(logoutUser())}>
238+
Log Out
239+
</NavMenuItem>
240+
</ParentMenuContext.Provider>
243241
</ul>
244242
</div>
245243
);
@@ -256,80 +254,55 @@ const MoreMenu = () => {
256254

257255
const cmRef = useContext(CmControllerContext);
258256

257+
const { isOpen, handlers } = useMenuProps('more');
258+
259259
return (
260260
<div>
261-
<IconButton icon={MoreIcon} />
262-
<ul>
263-
<b>{t('Nav.File.Title')}</b>
264-
<li>
265-
<button onClick={newSketch}>{t('Nav.File.New')}</button>
266-
</li>
267-
<li>
268-
<button onClick={() => saveSketch(cmRef.current)}>
261+
<IconButton icon={MoreIcon} {...handlers} />
262+
<ul className={isOpen ? 'opened' : ''}>
263+
<ParentMenuContext.Provider value="more">
264+
<b>{t('Nav.File.Title')}</b>
265+
<NavMenuItem onClick={newSketch}>{t('Nav.File.New')}</NavMenuItem>
266+
267+
<NavMenuItem onClick={() => saveSketch(cmRef.current)}>
269268
{t('Common.Save')}
270-
</button>
271-
</li>
272-
<li>
273-
<Link to="/p5/sketches">
274-
<button>{t('Nav.File.Examples')}</button>
275-
</Link>
276-
</li>
277-
<b>{t('Nav.Edit.Title')}</b>
278-
<li>
279-
<button onClick={cmRef.current?.tidyCode}>
269+
</NavMenuItem>
270+
<NavMenuItem href="/p5/sketches">
271+
{t('Nav.File.Examples')}
272+
</NavMenuItem>
273+
<b>{t('Nav.Edit.Title')}</b>
274+
<NavMenuItem onClick={cmRef.current?.tidyCode}>
280275
{t('Nav.Edit.TidyCode')}
281-
</button>
282-
</li>
283-
<li>
284-
<button onClick={cmRef.current?.showFind}>
276+
</NavMenuItem>
277+
<NavMenuItem onClick={cmRef.current?.showFind}>
285278
{t('Nav.Edit.Find')}
286-
</button>
287-
</li>
288-
<b>{t('Nav.Sketch.Title')}</b>
289-
<li>
290-
<button onClick={() => dispatch(newFile(rootFile.id))}>
279+
</NavMenuItem>
280+
<b>{t('Nav.Sketch.Title')}</b>
281+
<NavMenuItem onClick={() => dispatch(newFile(rootFile.id))}>
291282
{t('Nav.Sketch.AddFile')}
292-
</button>
293-
</li>
294-
<li>
295-
<button onClick={() => dispatch(newFolder(rootFile.id))}>
283+
</NavMenuItem>
284+
<NavMenuItem onClick={() => dispatch(newFolder(rootFile.id))}>
296285
{t('Nav.Sketch.AddFolder')}
297-
</button>
298-
</li>
299-
{/* TODO: Add Translations */}
300-
<b>Settings</b>
301-
<li>
302-
<button
286+
</NavMenuItem>
287+
{/* TODO: Add Translations */}
288+
<b>Settings</b>
289+
<NavMenuItem
303290
onClick={() => {
304291
dispatch(openPreferences());
305292
}}
306293
>
307294
Preferences
308-
</button>
309-
</li>
310-
<li>
311-
<button>Language</button>
312-
</li>
313-
<b>{t('Nav.Help.Title')}</b>
314-
<li>
315-
<button onClick={() => dispatch(showKeyboardShortcutModal())}>
295+
</NavMenuItem>
296+
<NavMenuItem>Language</NavMenuItem>
297+
<b>{t('Nav.Help.Title')}</b>
298+
<NavMenuItem onClick={() => dispatch(showKeyboardShortcutModal())}>
316299
{t('Nav.Help.KeyboardShortcuts')}
317-
</button>
318-
</li>
319-
<li>
320-
<button
321-
onClick={() => {
322-
window.location = 'https://p5js.org/reference/';
323-
}}
324-
>
300+
</NavMenuItem>
301+
<NavMenuItem href="https://p5js.org/reference/">
325302
{t('Nav.Help.Reference')}
326-
</button>
327-
</li>
328-
<li>
329-
<Link to="/about">
330-
<button>{t('Nav.Help.About')}</button>
331-
</Link>
332-
</li>
303+
</NavMenuItem>
304+
<NavMenuItem href="/about">{t('Nav.Help.About')}</NavMenuItem>
305+
</ParentMenuContext.Provider>
333306
</ul>
334307
</div>
335308
);

‎client/modules/IDE/components/Header/Nav.unit.test.jsx

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,30 @@ import { reduxRender } from '../../../../test-utils';
33

44
import Nav from './Nav';
55

6-
// jest.mock('../i18n');
6+
jest.mock('../../../../utils/generateRandomName');
77

88
describe('Nav', () => {
9-
it('renders editor version', () => {
10-
const { asFragment } = reduxRender(<Nav />);
9+
it('renders editor version for desktop', () => {
10+
const { asFragment } = reduxRender(<Nav />, { mobile: false });
1111
expect(asFragment()).toMatchSnapshot();
1212
});
1313

14-
it('renders dashboard version', () => {
15-
const { asFragment } = reduxRender(<Nav layout="dashboard" />);
14+
it('renders editor version for mobile', () => {
15+
const { asFragment } = reduxRender(<Nav />, { mobile: true });
16+
expect(asFragment()).toMatchSnapshot();
17+
});
18+
19+
it('renders dashboard version for desktop', () => {
20+
const { asFragment } = reduxRender(<Nav layout="dashboard" />, {
21+
mobile: false
22+
});
23+
expect(asFragment()).toMatchSnapshot();
24+
});
25+
26+
it('renders dashboard version for mobile', () => {
27+
const { asFragment } = reduxRender(<Nav layout="dashboard" />, {
28+
mobile: true
29+
});
1630
expect(asFragment()).toMatchSnapshot();
1731
});
1832
});

0 commit comments

Comments
 (0)