Skip to content

Commit a9a68a4

Browse files
committed
fix(ui-buttons): fix tabindex=0 added unnecessarly to Buttons
This is added by the browser for interactive element Also fix builds failing due to missing import in Breadcrumb INSTUI-4499
1 parent abad280 commit a9a68a4

File tree

5 files changed

+54
-5
lines changed

5 files changed

+54
-5
lines changed

package-lock.json

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/ui-breadcrumb/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
"@instructure/ui-react-utils": "10.15.1",
4242
"@instructure/ui-testable": "10.15.1",
4343
"@instructure/ui-truncate-text": "10.15.1",
44+
"@instructure/ui-tooltip": "10.15.1",
4445
"@instructure/ui-utils": "10.15.1",
4546
"@instructure/ui-view": "10.15.1",
4647
"prop-types": "^15.8.1"

packages/ui-breadcrumb/tsconfig.build.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
{ "path": "../ui-react-utils/tsconfig.build.json" },
1818
{ "path": "../ui-testable/tsconfig.build.json" },
1919
{ "path": "../ui-truncate-text/tsconfig.build.json" },
20+
{ "path": "../ui-tooltip/tsconfig.build.json" },
2021
{ "path": "../ui-utils/tsconfig.build.json" },
2122
{ "path": "../ui-view/tsconfig.build.json" },
2223
{ "path": "../ui-axe-check/tsconfig.build.json" },

packages/ui-buttons/src/BaseButton/__new-tests__/BaseButton.test.tsx

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,27 @@ describe('<BaseButton/>', () => {
126126
expect(button).toHaveAttribute('tabIndex', '0')
127127
})
128128

129+
it('should not set tabIndex="0" when the element has it by default', () => {
130+
const onClick = vi.fn()
131+
132+
render(
133+
<>
134+
<BaseButton as="button" onClick={onClick}>
135+
Hello Button
136+
</BaseButton>
137+
<BaseButton onClick={onClick} href="example.html">
138+
Hello link
139+
</BaseButton>
140+
</>
141+
)
142+
const button = screen.getByRole('button', { name: 'Hello Button' })
143+
expect(button).toBeInTheDocument()
144+
expect(button).not.toHaveAttribute('tabIndex')
145+
const link = screen.getByRole('link', { name: 'Hello link' })
146+
expect(link).toBeInTheDocument()
147+
expect(link).not.toHaveAttribute('tabIndex')
148+
})
149+
129150
it('should pass down the type prop to the button element', async () => {
130151
const onClick = vi.fn()
131152
render(
@@ -163,7 +184,7 @@ describe('<BaseButton/>', () => {
163184
expect(document.activeElement).toBe(button)
164185
})
165186

166-
it('should provide an elementRef prop', async () => {
187+
it('should provide an elementRef prop', async () => {
167188
const elementRef = vi.fn()
168189
render(<BaseButton elementRef={elementRef}>Hello World</BaseButton>)
169190

packages/ui-buttons/src/BaseButton/index.tsx

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,12 +255,37 @@ class BaseButton extends Component<BaseButtonProps> {
255255
...props
256256
} = this.props
257257

258-
const { isDisabled, isEnabled, isReadOnly } = this
259-
258+
const { isDisabled, isEnabled, isReadOnly, elementType } = this
259+
// only add 0 tabIndex value if it doesn't have it by default, see
260+
// https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex
261+
let needsZeroTabIndex = true
262+
if (typeof elementType == 'string') {
263+
if (
264+
[
265+
'button',
266+
'frame',
267+
'iframe',
268+
'input',
269+
'object',
270+
'select',
271+
'textarea',
272+
'summary'
273+
].includes(elementType)
274+
) {
275+
needsZeroTabIndex = false
276+
}
277+
if (href && (elementType === 'a' || elementType === 'area')) {
278+
needsZeroTabIndex = false
279+
}
280+
}
281+
let tabIndexValue = tabIndex
282+
if (onClick && as && needsZeroTabIndex) {
283+
tabIndexValue = tabIndex || 0
284+
}
260285
return (
261286
<View
262287
{...passthroughProps(props)}
263-
as={this.elementType}
288+
as={elementType}
264289
focusColor={this.focusColor}
265290
position="relative"
266291
display={display}
@@ -277,7 +302,7 @@ class BaseButton extends Component<BaseButtonProps> {
277302
onClick={this.handleClick}
278303
onKeyDown={this.handleKeyDown}
279304
role={onClick && as !== 'button' ? 'button' : undefined}
280-
tabIndex={onClick && as ? tabIndex || 0 : tabIndex}
305+
tabIndex={tabIndexValue}
281306
disabled={isDisabled || isReadOnly}
282307
css={isEnabled ? styles?.baseButton : null}
283308
focusRingBorderRadius={String(

0 commit comments

Comments
 (0)