Skip to content

Commit 79989dd

Browse files
authored
Merge pull request #3308 from ably/channel-rule-tabs-design-review
design improvements for rule creation tabs
2 parents 3f58156 + 03c6f20 commit 79989dd

File tree

4 files changed

+53
-98
lines changed

4 files changed

+53
-98
lines changed

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
"@radix-ui/react-accordion": "^1.2.12",
4949
"@radix-ui/react-dropdown-menu": "^2.1.16",
5050
"@radix-ui/react-select": "^2.2.6",
51+
"@radix-ui/react-tabs": "^1.1.13",
5152
"@radix-ui/react-tooltip": "^1.2.8",
5253
"@react-hook/media-query": "^1.1.1",
5354
"@sentry/gatsby": "^9.19.0",

src/components/Layout/mdx/Tabs.test.tsx

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import React from 'react';
2-
import { render, screen, fireEvent } from '@testing-library/react';
1+
import { render, screen } from '@testing-library/react';
2+
import userEvent from '@testing-library/user-event';
33
import { Tabs, Tab } from './Tabs';
44

55
describe('Tabs', () => {
@@ -31,11 +31,11 @@ describe('Tabs', () => {
3131
</Tabs>,
3232
);
3333

34-
expect(screen.getByText('Content A')).toBeInTheDocument();
35-
expect(screen.queryByText('Content B')).not.toBeInTheDocument();
34+
expect(screen.getByRole('tabpanel')).toHaveTextContent('Content A');
3635
});
3736

38-
it('switches content when a tab is clicked', () => {
37+
it('switches content when a tab is clicked', async () => {
38+
const user = userEvent.setup();
3939
render(
4040
<Tabs>
4141
<Tab value="a" label="Alpha">
@@ -47,13 +47,13 @@ describe('Tabs', () => {
4747
</Tabs>,
4848
);
4949

50-
fireEvent.click(screen.getByRole('tab', { name: 'Beta' }));
50+
await user.click(screen.getByRole('tab', { name: 'Beta' }));
5151

52-
expect(screen.queryByText('Content A')).not.toBeInTheDocument();
53-
expect(screen.getByText('Content B')).toBeInTheDocument();
52+
expect(screen.getByRole('tabpanel')).toHaveTextContent('Content B');
5453
});
5554

56-
it('sets aria-selected correctly', () => {
55+
it('sets aria-selected correctly', async () => {
56+
const user = userEvent.setup();
5757
render(
5858
<Tabs>
5959
<Tab value="a" label="Alpha">
@@ -68,7 +68,7 @@ describe('Tabs', () => {
6868
expect(screen.getByRole('tab', { name: 'Alpha' })).toHaveAttribute('aria-selected', 'true');
6969
expect(screen.getByRole('tab', { name: 'Beta' })).toHaveAttribute('aria-selected', 'false');
7070

71-
fireEvent.click(screen.getByRole('tab', { name: 'Beta' }));
71+
await user.click(screen.getByRole('tab', { name: 'Beta' }));
7272

7373
expect(screen.getByRole('tab', { name: 'Alpha' })).toHaveAttribute('aria-selected', 'false');
7474
expect(screen.getByRole('tab', { name: 'Beta' })).toHaveAttribute('aria-selected', 'true');
@@ -88,13 +88,4 @@ describe('Tabs', () => {
8888

8989
expect(screen.getByRole('tabpanel')).toHaveTextContent('Content A');
9090
});
91-
92-
it('renders nothing for Tab used outside of Tabs', () => {
93-
const { container } = render(
94-
<Tab value="a" label="Alpha">
95-
Orphan
96-
</Tab>,
97-
);
98-
expect(container).toBeEmptyDOMElement();
99-
});
10091
});

src/components/Layout/mdx/Tabs.tsx

Lines changed: 38 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,75 +1,63 @@
1-
import React, { useState, createContext, useContext, isValidElement, ReactNode, useId } from 'react';
1+
import React, { isValidElement, ReactNode } from 'react';
2+
import * as RadixTabs from '@radix-ui/react-tabs';
23
import cn from '@ably/ui/core/utils/cn';
34

4-
type TabsContextType = {
5-
activeTab: string;
6-
tabsId: string;
7-
};
8-
9-
const TabsContext = createContext<TabsContextType | undefined>(undefined);
10-
115
interface TabProps {
126
value: string;
137
label: string;
148
children: ReactNode;
159
}
1610

17-
export const Tab: React.FC<TabProps> = ({ value, children }) => {
18-
const context = useContext(TabsContext);
19-
if (!context) {
20-
return null;
21-
}
22-
return context.activeTab === value ? (
23-
<div role="tabpanel" id={`${context.tabsId}-panel-${value}`} aria-labelledby={`${context.tabsId}-tab-${value}`}>
24-
{children}
25-
</div>
26-
) : null;
11+
export const Tab: React.FC<TabProps> = () => {
12+
// Tab is only used declaratively — Tabs reads its props and renders RadixTabs.Content.
13+
// When used outside of Tabs, render nothing.
14+
return null;
2715
};
2816

2917
interface TabsProps {
3018
children: ReactNode;
3119
}
3220

3321
export const Tabs: React.FC<TabsProps> = ({ children }) => {
34-
const tabsId = useId();
35-
3622
const tabs: { value: string; label: string }[] = [];
23+
const contentByValue: Record<string, ReactNode> = {};
24+
3725
React.Children.forEach(children, (child) => {
3826
if (isValidElement<TabProps>(child) && child.props.value) {
3927
tabs.push({ value: child.props.value, label: child.props.label ?? child.props.value });
28+
contentByValue[child.props.value] = child.props.children;
4029
}
4130
});
4231

43-
const [activeTab, setActiveTab] = useState(tabs[0]?.value ?? '');
44-
4532
return (
46-
<TabsContext.Provider value={{ activeTab, tabsId }}>
47-
<div className="my-5 border border-neutral-300 dark:border-neutral-800 rounded-lg overflow-hidden">
48-
<div
49-
className="flex gap-1 border-b border-neutral-300 dark:border-neutral-800 bg-neutral-100 dark:bg-neutral-1100 px-2 pt-2"
50-
role="tablist"
51-
>
52-
{tabs.map(({ value, label }) => (
53-
<button
54-
key={value}
55-
id={`${tabsId}-tab-${value}`}
56-
role="tab"
57-
aria-selected={activeTab === value}
58-
aria-controls={`${tabsId}-panel-${value}`}
59-
onClick={() => setActiveTab(value)}
60-
className={cn(
61-
'px-4 py-2 text-sm font-medium rounded-t-md transition-colors cursor-pointer',
62-
activeTab === value
63-
? 'bg-white dark:bg-neutral-1300 text-neutral-1300 dark:text-neutral-000 border border-neutral-300 dark:border-neutral-800 border-b-white dark:border-b-neutral-1300 -mb-px'
64-
: 'text-neutral-700 dark:text-neutral-500 hover:text-neutral-1000 dark:hover:text-neutral-300',
65-
)}
66-
>
67-
{label}
68-
</button>
69-
))}
70-
</div>
71-
<div className="p-5">{children}</div>
72-
</div>
73-
</TabsContext.Provider>
33+
<RadixTabs.Root
34+
defaultValue={tabs[0]?.value}
35+
className="my-5 border border-neutral-300 dark:border-neutral-800 rounded-lg overflow-hidden"
36+
>
37+
<RadixTabs.List className="flex gap-1 border-b border-neutral-300 dark:border-neutral-800 bg-neutral-100 dark:bg-neutral-1100 px-2 pt-2">
38+
{tabs.map(({ value, label }) => (
39+
<RadixTabs.Trigger
40+
key={value}
41+
value={value}
42+
style={{ outline: 'none', borderTopLeftRadius: '6px', borderTopRightRadius: '6px' }}
43+
className={cn(
44+
'px-4 py-2 ui-text-label3 transition-colors cursor-pointer',
45+
'border border-transparent',
46+
'text-neutral-700 dark:text-neutral-500 hover:text-neutral-1000 dark:hover:text-neutral-300',
47+
'data-[state=active]:bg-white data-[state=active]:dark:bg-neutral-1300 data-[state=active]:text-neutral-1300 data-[state=active]:dark:text-neutral-000',
48+
'data-[state=active]:border-neutral-300 data-[state=active]:dark:border-neutral-800',
49+
'data-[state=active]:border-b-white data-[state=active]:dark:border-b-neutral-1300 data-[state=active]:-mb-px',
50+
)}
51+
>
52+
{label}
53+
</RadixTabs.Trigger>
54+
))}
55+
</RadixTabs.List>
56+
{tabs.map(({ value }) => (
57+
<RadixTabs.Content key={value} value={value} className="px-5 pt-5 pb-2.5">
58+
{contentByValue[value]}
59+
</RadixTabs.Content>
60+
))}
61+
</RadixTabs.Root>
7462
);
7563
};

yarn.lock

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3426,7 +3426,7 @@
34263426
"@radix-ui/react-use-previous" "1.1.1"
34273427
"@radix-ui/react-use-size" "1.1.1"
34283428

3429-
"@radix-ui/react-tabs@^1.1.1":
3429+
"@radix-ui/react-tabs@^1.1.1", "@radix-ui/react-tabs@^1.1.13":
34303430
version "1.1.13"
34313431
resolved "https://registry.yarnpkg.com/@radix-ui/react-tabs/-/react-tabs-1.1.13.tgz#3537ce379d7e7ff4eeb6b67a0973e139c2ac1f15"
34323432
integrity sha512-7xdcatg7/U+7+Udyoj2zodtI9H/IIopqo+YOIcZOq1nJwXWBZ9p8xiu5llXlekDbZkca79a/fozEYQXIA4sW6A==
@@ -14834,16 +14834,7 @@ string-similarity@^1.2.2:
1483414834
lodash.map "^4.6.0"
1483514835
lodash.maxby "^4.6.0"
1483614836

14837-
"string-width-cjs@npm:string-width@^4.2.0":
14838-
version "4.2.3"
14839-
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
14840-
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
14841-
dependencies:
14842-
emoji-regex "^8.0.0"
14843-
is-fullwidth-code-point "^3.0.0"
14844-
strip-ansi "^6.0.1"
14845-
14846-
string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3:
14837+
"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3:
1484714838
version "4.2.3"
1484814839
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
1484914840
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
@@ -14953,7 +14944,7 @@ stringify-entities@^4.0.0:
1495314944
character-entities-html4 "^2.0.0"
1495414945
character-entities-legacy "^3.0.0"
1495514946

14956-
"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
14947+
"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1:
1495714948
version "6.0.1"
1495814949
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
1495914950
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
@@ -14974,13 +14965,6 @@ strip-ansi@^5.2.0:
1497414965
dependencies:
1497514966
ansi-regex "^4.1.0"
1497614967

14977-
strip-ansi@^6.0.0, strip-ansi@^6.0.1:
14978-
version "6.0.1"
14979-
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
14980-
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
14981-
dependencies:
14982-
ansi-regex "^5.0.1"
14983-
1498414968
strip-ansi@^7.0.1:
1498514969
version "7.1.2"
1498614970
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.2.tgz#132875abde678c7ea8d691533f2e7e22bb744dba"
@@ -16430,7 +16414,7 @@ word-wrap@^1.2.5:
1643016414
resolved "https://registry.yarnpkg.com/word-wrap/-/word-wrap-1.2.5.tgz#d2c45c6dd4fbce621a66f136cbe328afd0410b34"
1643116415
integrity sha512-BN22B5eaMMI9UMtjrGd5g5eCYPpCPDUy0FJXbYsaT5zYxjFOckS53SQDE3pWkVoWpHXVb3BrYcEN4Twa55B5cA==
1643216416

16433-
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
16417+
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
1643416418
version "7.0.0"
1643516419
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
1643616420
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
@@ -16448,15 +16432,6 @@ wrap-ansi@^6.2.0:
1644816432
string-width "^4.1.0"
1644916433
strip-ansi "^6.0.0"
1645016434

16451-
wrap-ansi@^7.0.0:
16452-
version "7.0.0"
16453-
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
16454-
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
16455-
dependencies:
16456-
ansi-styles "^4.0.0"
16457-
string-width "^4.1.0"
16458-
strip-ansi "^6.0.0"
16459-
1646016435
wrap-ansi@^8.0.1, wrap-ansi@^8.1.0:
1646116436
version "8.1.0"
1646216437
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214"

0 commit comments

Comments
 (0)