Skip to content

Commit ea7f70e

Browse files
authored
chore: Provide correct role and href for disabled link buttons (#3685)
1 parent f8cc805 commit ea7f70e

File tree

3 files changed

+52
-1
lines changed

3 files changed

+52
-1
lines changed

src/button/__tests__/button.test.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ describe('Button Component', () => {
116116
expect(wrapper.getElement()).toHaveClass(styles.disabled);
117117
expect(wrapper.getElement()).not.toHaveAttribute('disabled');
118118
expect(wrapper.getElement()).toHaveAttribute('aria-disabled');
119+
expect(wrapper.getElement()).toHaveAttribute('role', 'link');
120+
expect(wrapper.getElement()).not.toHaveAttribute('href');
119121
expect(wrapper.isDisabled()).toBe(true);
120122
});
121123

@@ -552,6 +554,8 @@ describe('Button Component', () => {
552554
test('adds a tab index -1 to the link button', () => {
553555
const wrapper = renderButton({ loading: true, href: 'https://amazon.com' });
554556
expect(wrapper.getElement()).toHaveAttribute('tabIndex', '-1');
557+
expect(wrapper.getElement()).not.toHaveAttribute('href');
558+
expect(wrapper.getElement()).toHaveAttribute('role', 'link');
555559
});
556560
});
557561

@@ -619,6 +623,18 @@ describe('Button Component', () => {
619623
expect(wrapper.getElement()).toHaveAttribute('href', 'https://amazon.com');
620624
});
621625

626+
test('removes href attribute and adds role="link" when button is disabled', () => {
627+
const wrapper = renderButton({ href: 'https://amazon.com', disabled: true });
628+
expect(wrapper.getElement()).not.toHaveAttribute('href');
629+
expect(wrapper.getElement()).toHaveAttribute('role', 'link');
630+
});
631+
632+
test('removes href attribute and adds role="link" when button is loading', () => {
633+
const wrapper = renderButton({ href: 'https://amazon.com', loading: true });
634+
expect(wrapper.getElement()).not.toHaveAttribute('href');
635+
expect(wrapper.getElement()).toHaveAttribute('role', 'link');
636+
});
637+
622638
test.each(buttonTargetExpectations)('"target" property %s', (props, expectation) => {
623639
const wrapper = renderButton({ ...props });
624640
if (expectation) {

src/button/__tests__/internal.test.tsx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,18 @@ describe('Analytics', () => {
8080
);
8181
});
8282

83+
test('does not send an externalLinkInteracted metric when button is disabled', () => {
84+
const { container } = render(
85+
<AnalyticsFunnel funnelType="single-page" optionalStepNumbers={[]} totalFunnelSteps={1}>
86+
<Button iconName="external" href="https://example.com" disabled={true} />
87+
</AnalyticsFunnel>
88+
);
89+
act(() => void jest.runAllTimers());
90+
createWrapper(container).findButton()!.click();
91+
92+
expect(FunnelMetrics.externalLinkInteracted).not.toHaveBeenCalled();
93+
});
94+
8395
test('does not send an externalLinkInteracted metric when the button is not a link', () => {
8496
const onClick = jest.fn();
8597
const { container } = render(
@@ -115,6 +127,18 @@ describe('Analytics', () => {
115127
);
116128
});
117129

130+
test('does not send an externalLinkInteracted metric when button with target=_blank is disabled', () => {
131+
const { container } = render(
132+
<AnalyticsFunnel funnelType="single-page" optionalStepNumbers={[]} totalFunnelSteps={1}>
133+
<Button target="_blank" href="https://example.com" disabled={true} />
134+
</AnalyticsFunnel>
135+
);
136+
act(() => void jest.runAllTimers());
137+
createWrapper(container).findButton()!.click();
138+
139+
expect(FunnelMetrics.externalLinkInteracted).not.toHaveBeenCalled();
140+
});
141+
118142
test('does not send an externalLinkInteracted metric for non-external links', () => {
119143
const { container } = render(
120144
<AnalyticsFunnel funnelType="single-page" optionalStepNumbers={[]} totalFunnelSteps={1}>

src/button/internal.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,11 +330,22 @@ export const InternalButton = React.forwardRef(
330330
: undefined;
331331

332332
if (isAnchor) {
333+
const getAnchorTabIndex = () => {
334+
if (isNotInteractive) {
335+
// If disabled with a reason, make it focusable so users can access the tooltip
336+
// Otherwise, resolve to the default button props tabIndex.
337+
return disabledReason ? 0 : buttonProps.tabIndex;
338+
}
339+
return buttonProps.tabIndex;
340+
};
341+
333342
return (
334343
<>
335344
<a
336345
{...buttonProps}
337-
href={href}
346+
href={isNotInteractive ? undefined : href}
347+
role={isNotInteractive ? 'link' : undefined}
348+
tabIndex={getAnchorTabIndex()}
338349
target={target}
339350
// security recommendation: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#target
340351
rel={rel ?? (target === '_blank' ? 'noopener noreferrer' : undefined)}

0 commit comments

Comments
 (0)