Skip to content
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

fix(react-link): support Enter and Space keys interaction, if rendered as span #33587

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
Copy link
Collaborator

@fabricteam fabricteam Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕵🏾‍♀️ visual regressions to review in the fluentuiv9 Visual Regression Report

Avatar Converged 2 screenshots
Image Name Diff(in Pixels) Image Type
Avatar Converged.Badge Mask RTL.chromium.png 19 Changed
Avatar Converged.badgeMask.chromium.png 3 Changed
Drawer 2 screenshots
Image Name Diff(in Pixels) Image Type
Drawer.Full Overlay High Contrast.chromium.png 2247 Changed
Drawer.Full Overlay RTL.chromium.png 1167 Changed

"type": "patch",
"comment": "fix: support Enter and Space interaction, if rendered as span",
"packageName": "@fluentui/react-link",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import * as React from 'react';
import { render } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { linkBehaviorDefinition, validateBehavior, ComponentTestFacade } from '@fluentui/a11y-testing';
import { isConformant } from '../../testing/isConformant';
import { Link } from './Link';
import { LinkProps } from './Link.types';
import { Enter } from '@fluentui/keyboard-keys';

describe('Link', () => {
isConformant<LinkProps>({
Expand Down Expand Up @@ -140,5 +142,45 @@ describe('Link', () => {
expect(result.queryAllByRole('link')).toHaveLength(0);
expect(result.queryAllByRole('presentation')).toHaveLength(1);
});

describe('when rendered as span', () => {
it('should call onClick by pressing Enter', () => {
const onClick = jest.fn();

render(
<Link as="span" onClick={onClick}>
This is a buttonlink
</Link>,
);

userEvent.tab();
userEvent.keyboard('{Enter}');

expect(onClick).toHaveBeenCalledTimes(1);
});

it('should trigger onClick once, if onKeyDown is provided', () => {
const onClick = jest.fn();

render(
<Link
as="span"
onClick={onClick}
onKeyDown={ev => {
if (ev.key === Enter) {
ev.currentTarget.click();
}
}}
>
This is a buttonlink
</Link>,
);

userEvent.tab();
userEvent.keyboard('{Enter}');

expect(onClick).toHaveBeenCalledTimes(1);
});
});
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import { Enter, Space } from '@fluentui/keyboard-keys';
import type { LinkState } from './Link.types';
import { mergeCallbacks } from '@fluentui/react-utilities';

/**
* The useLinkState_unstable hook processes the Link state.
Expand Down Expand Up @@ -35,14 +36,24 @@ export const useLinkState_unstable = (state: LinkState): LinkState => {
};

// Disallow keydown event when component is disabled and eat events when disabledFocusable is set to true.
state.root.onKeyDown = (ev: React.KeyboardEvent<HTMLAnchorElement & HTMLButtonElement>) => {
if ((disabled || disabledFocusable) && (ev.key === Enter || ev.key === Space)) {
ev.preventDefault();
ev.stopPropagation();
} else {
onKeyDown?.(ev);
}
};
state.root.onKeyDown = mergeCallbacks(
state.root.onKeyDown,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wont this break current behaviour when disabled ? this merging callback will invoke provided onKeyDown then the custom function ( only the custom function contains disabling resolution behaviours )

(ev: React.KeyboardEvent<HTMLAnchorElement & HTMLButtonElement>) => {
const keyPressed = ev.key === Enter || ev.key === Space;

if ((disabled || disabledFocusable) && keyPressed) {
ev.preventDefault();
ev.stopPropagation();
return;
}

// if there is already onKeyDown provided - respect it
if (state.root.as === 'span' && !!state.root.onClick && !onKeyDown && keyPressed) {
ev.preventDefault();
ev.currentTarget.click();
}
},
);

// Set the aria-disabled and disabled props correctly.
state.disabled = disabled || disabledFocusable;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const useDivWithWidthClassName = makeResetStyles({
export const AsSpan = () => (
<div className={useDivWithWidthClassName()}>
The following link renders as a span.{' '}
<Link as="span" inline>
<Link as="span" inline onClick={() => alert('Link rendered as span')}>
Links that render as a span wrap correctly between lines when their content is very long
</Link>
. This is because they behave as regular inline elements.
Expand Down
Loading