Skip to content

Commit 0996d8f

Browse files
geotrevgeotrev
authored
feat(chrome)!: improves markup structure for Nav items (#1784)
1 parent 04bad34 commit 0996d8f

File tree

17 files changed

+270
-117
lines changed

17 files changed

+270
-117
lines changed

‎docs/migration.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ consider additional positioning prop support on a case-by-case basis.
7676
- `CollapsibleSubNavItem` -> `SubNav.CollapsibleItem`
7777
- `SubNavItem` -> `SubNav.Item`
7878
- `SubNavItemText` -> `SubNav.ItemText`
79+
- Added `Nav.List` as a semantic wrapper for `Nav.Item`. See the
80+
[README](../packages/chrome/README.md#usages) for details.
7981

8082
#### @zendeskgarden/react-colorpickers
8183

‎packages/avatars/demo/~patterns/stories/ChromeStory.tsx‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,23 @@
88
import React from 'react';
99
import { Story } from '@storybook/react';
1010
import Icon from '@zendeskgarden/svg-icons/src/16/grid-2x2-stroke.svg';
11-
import { Chrome, Body, Header, HeaderItem, HeaderItemIcon } from '@zendeskgarden/react-chrome';
11+
import { Chrome, Body, Header } from '@zendeskgarden/react-chrome';
1212
import { Avatar, IAvatarProps } from '@zendeskgarden/react-avatars';
1313

1414
export const ChromeStory: Story<IAvatarProps> = args => (
1515
<Chrome isFluid style={{ height: 'auto' }}>
1616
<Body>
1717
<Header>
18-
<HeaderItem aria-label="Products">
19-
<HeaderItemIcon>
18+
<Header.Item aria-label="Products">
19+
<Header.ItemIcon>
2020
<Icon />
21-
</HeaderItemIcon>
22-
</HeaderItem>
23-
<HeaderItem isRound aria-label="User profile">
21+
</Header.ItemIcon>
22+
</Header.Item>
23+
<Header.Item isRound aria-label="User profile">
2424
<Avatar {...args} size="extrasmall">
2525
<img alt="Example User" src="images/avatars/chrome.png" />
2626
</Avatar>
27-
</HeaderItem>
27+
</Header.Item>
2828
</Header>
2929
</Body>
3030
</Chrome>

‎packages/chrome/README.md‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ npm install react react-dom styled-components @zendeskgarden/react-theming
1717
import { ThemeProvider } from '@zendeskgarden/react-theming';
1818
import { Chrome, Nav, SubNav, Body, Header, Content, Main } from '@zendeskgarden/react-chrome';
1919
import ConnectIcon from '@zendeskgarden/icons/src/26/relationshape-connect.svg';
20+
import BrandmarkIcon from '@zendeskgarden/svg-icons/src/26/zendesk.svg';
2021

2122
<ThemeProvider>
2223
<Chrome>
@@ -27,11 +28,19 @@ import ConnectIcon from '@zendeskgarden/icons/src/26/relationshape-connect.svg';
2728
</Nav.ItemIcon>
2829
<NavItemText>Zendesk Connect</NavItemText>
2930
</Nav.Item>
30-
<Nav.Item isCurrent>
31+
<Nav.List>
32+
<Nav.Item isCurrent>
33+
<Nav.ItemIcon>
34+
<HomeIcon />
35+
</Nav.ItemIcon>
36+
<NavItemText>Home</NavItemText>
37+
</Nav.Item>
38+
</Nav.List>
39+
<Nav.Item hasBrandmark>
3140
<Nav.ItemIcon>
32-
<HomeIcon />
41+
<BrandmarkIcon />
3342
</Nav.ItemIcon>
34-
<NavItemText>Home</NavItemText>
43+
<Nav.ItemText>Brandmark</Nav.ItemText>
3544
</Nav.Item>
3645
</Nav>
3746
<SubNav>

‎packages/chrome/demo/stories/ChromeStory.tsx‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -139,20 +139,22 @@ export const ChromeStory: Story<IArgs> = ({
139139
<Nav.ItemText>Nav Logo</Nav.ItemText>
140140
</Nav.Item>
141141
)}
142-
{navItems.map((item, index) => (
143-
<Nav.Item
144-
key={index}
145-
isCurrent={currentNav === index}
146-
onClick={() => {
147-
setCurrentNav(index);
148-
setCurrentSubNav(0);
149-
onNavClick({ hasSubNav: item.hasSubNav });
150-
}}
151-
>
152-
<Nav.ItemIcon>{NAV_ICONS[index] || <NavIcon />}</Nav.ItemIcon>
153-
<Nav.ItemText isWrapped={isWrapped}>{item.text}</Nav.ItemText>
154-
</Nav.Item>
155-
))}
142+
<Nav.List>
143+
{navItems.map((item, index) => (
144+
<Nav.Item
145+
key={index}
146+
isCurrent={currentNav === index}
147+
onClick={() => {
148+
setCurrentNav(index);
149+
setCurrentSubNav(0);
150+
onNavClick({ hasSubNav: item.hasSubNav });
151+
}}
152+
>
153+
<Nav.ItemIcon>{NAV_ICONS[index] || <NavIcon />}</Nav.ItemIcon>
154+
<Nav.ItemText isWrapped={isWrapped}>{item.text}</Nav.ItemText>
155+
</Nav.Item>
156+
))}
157+
</Nav.List>
156158
{hasBrandmark && (
157159
<Nav.Item hasBrandmark>
158160
<Nav.ItemIcon>

‎packages/chrome/src/elements/nav/Nav.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { StyledNav } from '../../styled';
1414
import { NavItem } from './NavItem';
1515
import { NavItemIcon } from './NavItemIcon';
1616
import { NavItemText } from './NavItemText';
17+
import { NavList } from './NavList';
1718

1819
export const NavComponent = React.forwardRef<HTMLElement, INavProps>((props, ref) => {
1920
const { hue, isLight, isDark } = useChromeContext();
@@ -36,11 +37,13 @@ NavComponent.propTypes = {
3637
* @extends HTMLAttributes<HTMLElement>
3738
*/
3839
export const Nav = NavComponent as typeof NavComponent & {
40+
List: typeof NavList;
3941
Item: typeof NavItem;
4042
ItemIcon: typeof NavItemIcon;
4143
ItemText: typeof NavItemText;
4244
};
4345

46+
Nav.List = NavList;
4447
Nav.Item = NavItem;
4548
Nav.ItemIcon = NavItemIcon;
4649
Nav.ItemText = NavItemText;

‎packages/chrome/src/elements/nav/NavItem.spec.tsx‎

Lines changed: 63 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,42 +9,55 @@ import React from 'react';
99
import { render } from 'garden-test-utils';
1010
import { PALETTE, getColorV8, DEFAULT_THEME } from '@zendeskgarden/react-theming';
1111
import { Chrome } from '../Chrome';
12-
import { NavItem } from './NavItem';
1312
import { Nav } from './Nav';
1413
import { PRODUCTS, Product } from '../../types';
1514

1615
describe('NavItem', () => {
1716
it('passes ref to underlying DOM element', () => {
1817
const ref = React.createRef<HTMLButtonElement>();
19-
const { container } = render(<NavItem ref={ref} />);
18+
const { getByTestId } = render(
19+
<Nav.List>
20+
<Nav.Item ref={ref} data-test-id="item" />
21+
</Nav.List>
22+
);
2023

21-
expect(container.firstChild).toBe(ref.current);
24+
expect(getByTestId('item')).toBe(ref.current);
2225
});
2326

2427
it('renders expanded styling', () => {
25-
const { container } = render(
28+
const { getByTestId } = render(
2629
<Nav isExpanded>
27-
<NavItem />
30+
<Nav.List>
31+
<Nav.Item data-test-id="item" />
32+
</Nav.List>
2833
</Nav>
2934
);
3035

31-
expect(container.firstChild!.firstChild).toHaveStyle(`
36+
expect(getByTestId('item')).toHaveStyle(`
3237
justify-content: start;
3338
text-align: inherit;
3439
`);
3540
});
3641

3742
describe('Current', () => {
3843
it('renders state attribute when current', () => {
39-
const { getByTestId } = render(<NavItem isCurrent data-test-id="current-nav-item" />);
44+
const { getByTestId } = render(
45+
<Nav.List>
46+
<Nav.Item isCurrent data-test-id="current-nav-item" />
47+
</Nav.List>
48+
);
4049

4150
const currentNavItem = getByTestId('current-nav-item');
4251

4352
expect(currentNavItem).toHaveAttribute('aria-current', 'true');
4453
});
4554

4655
it('does not render state attribute when not current', () => {
47-
const { getByTestId } = render(<NavItem data-test-id="nav-item" />);
56+
const { getByTestId } = render(
57+
<Nav.List>
58+
<Nav.Item data-test-id="nav-item" />
59+
</Nav.List>
60+
);
4861

4962
const navItem = getByTestId('nav-item');
5063

@@ -54,110 +67,110 @@ describe('NavItem', () => {
5467

5568
describe('Order', () => {
5669
it('renders correct order if used as brandmark', () => {
57-
const { container } = render(<NavItem hasBrandmark />);
70+
const { container } = render(<Nav.Item hasBrandmark />);
5871

5972
expect(container.firstChild).toHaveStyleRule('order', '1');
6073
});
6174

6275
it('renders correct order if used as logo', () => {
63-
const { container } = render(<NavItem hasLogo />);
76+
const { container } = render(<Nav.Item hasLogo />);
6477

65-
expect(container.firstChild).toHaveStyleRule('order', '0');
78+
expect(container.firstChild).toHaveStyleRule('order', '-1');
6679
});
6780
});
6881

6982
describe('Opacity', () => {
7083
it('renders correct opacity if used as brandmark', () => {
71-
const { container } = render(<NavItem hasBrandmark />);
84+
const { container } = render(<Nav.Item hasBrandmark />);
7285

7386
expect(container.firstChild).toHaveStyleRule('opacity', '0.3');
7487
});
7588

7689
it('renders correct opacity if used as logo', () => {
77-
const { container } = render(<NavItem hasLogo />);
90+
const { container } = render(<Nav.Item hasLogo />);
7891

7992
expect(container.firstChild).toHaveStyleRule('opacity', '1');
8093
});
8194

8295
it('renders correct opacity if current', () => {
83-
const { container } = render(<NavItem isCurrent />);
96+
const { getByTestId } = render(
97+
<Nav.List>
98+
<Nav.Item isCurrent data-test-id="item" />
99+
</Nav.List>
100+
);
84101

85-
expect(container.firstChild).toHaveStyleRule('opacity', '1');
102+
expect(getByTestId('item')).toHaveStyleRule('opacity', '1');
86103
});
87104
});
88105

89106
describe('Hover Color', () => {
90107
it('renders correct color with dark hue', () => {
91-
const { container } = render(
108+
const { getByTestId } = render(
92109
<Chrome hue="black">
93-
<NavItem />
110+
<Nav.List>
111+
<Nav.Item data-test-id="item" />
112+
</Nav.List>
94113
</Chrome>
95114
);
96115

97-
expect(container.firstChild!.firstChild).toHaveStyleRule(
98-
'background-color',
99-
'rgba(0,0,0,0.1)',
100-
{
101-
modifier: '&:hover'
102-
}
103-
);
116+
expect(getByTestId('item')).toHaveStyleRule('background-color', 'rgba(0,0,0,0.1)', {
117+
modifier: '&:hover'
118+
});
104119
});
105120

106121
it('renders correct color with light hue', () => {
107-
const { container } = render(
122+
const { getByTestId } = render(
108123
<Chrome hue="white">
109-
<NavItem />
124+
<Nav.List>
125+
<Nav.Item data-test-id="item" />
126+
</Nav.List>
110127
</Chrome>
111128
);
112129

113-
expect(container.firstChild!.firstChild).toHaveStyleRule(
114-
'background-color',
115-
'rgba(255,255,255,0.1)',
116-
{
117-
modifier: '&:hover'
118-
}
119-
);
130+
expect(getByTestId('item')).toHaveStyleRule('background-color', 'rgba(255,255,255,0.1)', {
131+
modifier: '&:hover'
132+
});
120133
});
121134
});
122135

123136
describe('Current Color', () => {
124137
it('renders correct color by default', () => {
125-
const { container } = render(
138+
const { getByTestId } = render(
126139
<Chrome>
127-
<NavItem isCurrent />
140+
<Nav.List>
141+
<Nav.Item isCurrent data-test-id="item" />
142+
</Nav.List>
128143
</Chrome>
129144
);
130145

131-
expect(container.firstChild!.firstChild).toHaveStyleRule(
146+
expect(getByTestId('item')).toHaveStyleRule(
132147
'background-color',
133148
getColorV8('chromeHue', 500, DEFAULT_THEME)
134149
);
135150
});
136151

137152
it('renders correct color with dark hue', () => {
138-
const { container } = render(
153+
const { getByTestId } = render(
139154
<Chrome hue="black">
140-
<NavItem isCurrent />
155+
<Nav.List>
156+
<Nav.Item isCurrent data-test-id="item" />
157+
</Nav.List>
141158
</Chrome>
142159
);
143160

144-
expect(container.firstChild!.firstChild).toHaveStyleRule(
145-
'background-color',
146-
'rgba(255,255,255,0.4)'
147-
);
161+
expect(getByTestId('item')).toHaveStyleRule('background-color', 'rgba(255,255,255,0.4)');
148162
});
149163

150164
it('renders correct color with light hue', () => {
151-
const { container } = render(
165+
const { getByTestId } = render(
152166
<Chrome hue="white">
153-
<NavItem isCurrent />
167+
<Nav.List>
168+
<Nav.Item isCurrent data-test-id="item" />
169+
</Nav.List>
154170
</Chrome>
155171
);
156172

157-
expect(container.firstChild!.firstChild).toHaveStyleRule(
158-
'background-color',
159-
'rgba(0,0,0,0.4)'
160-
);
173+
expect(getByTestId('item')).toHaveStyleRule('background-color', 'rgba(0,0,0,0.4)');
161174
});
162175
});
163176

@@ -174,14 +187,14 @@ describe('NavItem', () => {
174187

175188
it('renders correct product color if provided', () => {
176189
PRODUCTS.forEach(product => {
177-
const { container } = render(<NavItem hasLogo product={product} />);
190+
const { container } = render(<Nav.Item hasLogo product={product} />);
178191

179192
expect(container.firstChild).toHaveStyleRule('color', VALID_COLOR_MAP[product]);
180193
});
181194
});
182195

183196
it('renders correct color if no product is provided', () => {
184-
const { container } = render(<NavItem hasLogo />);
197+
const { container } = render(<Nav.Item hasLogo />);
185198

186199
expect(container.firstChild).toHaveStyleRule('color', 'inherit');
187200
});

0 commit comments

Comments
 (0)