-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Tab
and TabList
components
#813
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
import classnames from 'classnames'; | ||
import type { JSX } from 'preact'; | ||
|
||
import type { IconComponent, PresentationalProps } from '../../types'; | ||
import { downcastRef } from '../../util/typing'; | ||
|
||
import ButtonBase from '../input/ButtonBase'; | ||
|
||
type ComponentProps = { | ||
icon?: IconComponent; | ||
/** | ||
* Text string representing the content of the tab when selected. The tab | ||
* button will be sized to accommodate this string in bold text. This can | ||
* prevent tab jiggle. | ||
*/ | ||
textContent?: string; | ||
selected?: boolean; | ||
variant?: 'basic'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand the intention is to have more There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was on the fence about this. Tell you what: with this UI prototyping, I'm exercising these components quite a lot (it led me to add icon support to |
||
}; | ||
|
||
type HTMLAttributes = Omit< | ||
JSX.HTMLAttributes<HTMLButtonElement>, | ||
'size' | 'icon' | 'title' | ||
>; | ||
|
||
export type TabProps = PresentationalProps & ComponentProps & HTMLAttributes; | ||
|
||
/** | ||
* Render a button with appropriate ARIA tab affordances | ||
*/ | ||
const TabNext = function Tab({ | ||
children, | ||
classes, | ||
elementRef, | ||
|
||
icon: Icon, | ||
textContent, | ||
selected = false, | ||
variant = 'basic', | ||
|
||
...htmlAttributes | ||
}: TabProps) { | ||
return ( | ||
<ButtonBase | ||
{...htmlAttributes} | ||
classes={classnames( | ||
'gap-x-1.5 enabled:hover:text-brand-dark', | ||
{ | ||
'font-bold': selected && variant === 'basic', | ||
}, | ||
classes | ||
)} | ||
elementRef={downcastRef(elementRef)} | ||
aria-selected={selected} | ||
role="tab" | ||
data-component="Tab" | ||
> | ||
{Icon && ( | ||
<Icon | ||
className={classnames( | ||
// A small padding value here sizes the icon down slightly in relation | ||
// to the tab text, which results in nicer proportions. | ||
'p-[0.125em] w-em h-em' | ||
)} | ||
/> | ||
)} | ||
<span | ||
data-content={textContent} | ||
data-testid="sizing-wrapper" | ||
className={classnames({ | ||
// Set the content of this span's ::before pseudo-element to | ||
// `textContent` and make it bold. | ||
'before:content-[attr(data-content)] before:font-bold': textContent, | ||
// Make the ::before occupy space within the button, but make it | ||
// invisible. This ensures that the tab button is wide enough to show | ||
// bolded text even if the visible text is not currently bold. See | ||
// https://css-tricks.com/bold-on-hover-without-the-layout-shift/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clever! 😃 |
||
'before:block before:invisible before:h-0 before:overflow-hidden': | ||
textContent, | ||
})} | ||
> | ||
{children} | ||
</span> | ||
</ButtonBase> | ||
); | ||
}; | ||
|
||
export default TabNext; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
import classnames from 'classnames'; | ||
import type { JSX } from 'preact'; | ||
|
||
import { useArrowKeyNavigation } from '../../hooks/use-arrow-key-navigation'; | ||
import { useSyncedRef } from '../../hooks/use-synced-ref'; | ||
import type { PresentationalProps } from '../../types'; | ||
import { downcastRef } from '../../util/typing'; | ||
|
||
type HTMLAttributes = Omit<JSX.HTMLAttributes<HTMLElement>, 'size'>; | ||
|
||
type ComponentProps = { | ||
/** | ||
* By default, TabLists are oriented horizontally. Vertically-oriented | ||
* TabLists add up/down arrow-key navigation. | ||
*/ | ||
vertical?: boolean; | ||
}; | ||
|
||
export type TabListProps = PresentationalProps & | ||
ComponentProps & | ||
HTMLAttributes; | ||
|
||
/** | ||
* Render a tablist container for a set of tabs, with arrow key navigation per | ||
* https://www.w3.org/WAI/ARIA/apg/patterns/tabpanel/ | ||
*/ | ||
const TabListNext = function TabList({ | ||
children, | ||
classes, | ||
elementRef, | ||
|
||
vertical = false, | ||
|
||
...htmlAttributes | ||
}: TabListProps) { | ||
const tabListRef = useSyncedRef(elementRef); | ||
|
||
useArrowKeyNavigation(tabListRef, { | ||
selector: 'button', | ||
horizontal: true, | ||
vertical, | ||
}); | ||
|
||
return ( | ||
<div | ||
{...htmlAttributes} | ||
ref={downcastRef(tabListRef)} | ||
className={classnames('flex', { 'flex-col': vertical }, classes)} | ||
role="tablist" | ||
aria-orientation={vertical ? 'vertical' : 'horizontal'} | ||
data-component="TabList" | ||
> | ||
{children} | ||
</div> | ||
); | ||
}; | ||
|
||
export default TabListNext; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import { mount } from 'enzyme'; | ||
|
||
import { testPresentationalComponent } from '../../test/common-tests'; | ||
|
||
import { ProfileIcon } from '../../icons'; | ||
import Tab from '../Tab'; | ||
|
||
const contentFn = (Component, props = {}) => { | ||
return mount( | ||
<div role="tablist"> | ||
<Component {...props}>This is child content</Component> | ||
</div> | ||
); | ||
}; | ||
Comment on lines
+8
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and in the tests for |
||
|
||
describe('Tab', () => { | ||
testPresentationalComponent(Tab, { | ||
createContent: contentFn, | ||
elementSelector: 'button[data-component="Tab"]', | ||
}); | ||
|
||
it('sets `aria-selected` when selected', () => { | ||
const tab1 = contentFn(Tab, { selected: true }); | ||
const tab2 = contentFn(Tab, { selected: false }); | ||
|
||
assert.equal( | ||
tab1.find('button').getDOMNode().getAttribute('aria-selected'), | ||
'true' | ||
); | ||
assert.equal( | ||
tab2.find('button').getDOMNode().getAttribute('aria-selected'), | ||
'false' | ||
); | ||
}); | ||
|
||
it('sets content data attribute on sizing span when `textContent` provided', () => { | ||
const wrapper = contentFn(Tab, { textContent: 'Tab Label' }); | ||
assert.equal( | ||
wrapper | ||
.find('[data-testid="sizing-wrapper"]') | ||
.getDOMNode() | ||
.getAttribute('data-content'), | ||
'Tab Label' | ||
); | ||
}); | ||
|
||
it('renders an icon when provided', () => { | ||
const wrapper = contentFn(Tab, { icon: ProfileIcon }); | ||
|
||
assert.isTrue(wrapper.find('ProfileIcon').exists()); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
import { mount } from 'enzyme'; | ||
|
||
import { testPresentationalComponent } from '../../test/common-tests'; | ||
|
||
import TabList from '../TabList'; | ||
import { $imports } from '../TabList'; | ||
|
||
/** | ||
* An element with `role="tablist"` needs at least one `role="tab"` child. | ||
* Accessibility tests will fail without it. | ||
*/ | ||
const contentFn = (Component, props = {}) => { | ||
return mount( | ||
<Component {...props}> | ||
<button role="tab">Tab 1</button> | ||
</Component> | ||
); | ||
}; | ||
|
||
describe('TabList', () => { | ||
testPresentationalComponent(TabList, { createContent: contentFn }); | ||
|
||
describe('TabList orientation and keyboard navigation', () => { | ||
let fakeUseArrowKeyNavigation; | ||
|
||
beforeEach(() => { | ||
fakeUseArrowKeyNavigation = sinon.stub(); | ||
$imports.$mock({ | ||
'../../hooks/use-arrow-key-navigation': { | ||
useArrowKeyNavigation: fakeUseArrowKeyNavigation, | ||
}, | ||
}); | ||
}); | ||
|
||
afterEach(() => { | ||
$imports.$restore(); | ||
}); | ||
|
||
it('sets `aria-orientation` to `horizontal` or `vertical` based on `vertical` prop', () => { | ||
const horizontalTabList = contentFn(TabList, {}); | ||
const verticalTabList = contentFn(TabList, { vertical: true }); | ||
|
||
assert.equal( | ||
horizontalTabList | ||
.find('[data-component="TabList"]') | ||
.getDOMNode() | ||
.getAttribute('aria-orientation'), | ||
'horizontal' | ||
); | ||
assert.equal( | ||
verticalTabList | ||
.find('[data-component="TabList"]') | ||
.getDOMNode() | ||
.getAttribute('aria-orientation'), | ||
'vertical' | ||
); | ||
}); | ||
|
||
it('applies horizontal (left/right) keyboard navigation when horizontal', () => { | ||
contentFn(TabList, {}); | ||
|
||
const navOpts = fakeUseArrowKeyNavigation.getCall(0).args[1]; | ||
|
||
assert.include(navOpts, { horizontal: true, vertical: false }); | ||
}); | ||
|
||
it('applies horizontal and vertical (up/down) keyboard navigation when vertical', () => { | ||
contentFn(TabList, { vertical: true }); | ||
|
||
const navOpts = fakeUseArrowKeyNavigation.getCall(0).args[1]; | ||
|
||
assert.include(navOpts, { horizontal: true, vertical: true }); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,4 +41,6 @@ export { | |
Link, | ||
LinkBase, | ||
LinkButton, | ||
Tab, | ||
TabList, | ||
} from './components/navigation/'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we could try to simplify the API by determining the
textContent
automatically, eg. in auseLayoutEffect
that readstextContent
after render, although that would have a limitation of not being able to account for expected text changes (eg. if counts were not available and become available later), and be a bit more expensive.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm going to continue to hem and haw on this some. I don't plan to immediately cut a release of the package, so something intelligent might occur to me in the next few days.