Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions .changeset/empty-snails-peel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@rocket.chat/fuselage-forms": patch
"@rocket.chat/fuselage-hooks": patch
---

fix(fuselage-forms): Clicking on label does not focus on input
4 changes: 2 additions & 2 deletions packages/fuselage-forms/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@
Firstly, install the peer dependencies (prerequisites):

```sh
npm i @rocket.chat/fuselage react react-dom
npm i @rocket.chat/fuselage @rocket.chat/fuselage-hooks react react-dom

# or, if you are using yarn:

yarn add @rocket.chat/fuselage react react-dom
yarn add @rocket.chat/fuselage @rocket.chat/fuselage-hooks react react-dom
```

Add `@rocket.chat/fuselage-forms` as a dependency:
Expand Down
3 changes: 3 additions & 0 deletions packages/fuselage-forms/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"@storybook/types": "~8.6.14",
"@testing-library/jest-dom": "~6.6.3",
"@testing-library/react": "~16.3.0",
"@testing-library/user-event": "~14.6.1",
"@types/jest": "~29.5.14",
"@types/jest-axe": "~3.5.9",
"@types/react": "~18.3.23",
Expand All @@ -65,6 +66,7 @@
},
"peerDependencies": {
"@rocket.chat/fuselage": "*",
"@rocket.chat/fuselage-hooks": "workspace:~",
"react": "*",
"react-dom": "*"
},
Expand All @@ -75,6 +77,7 @@
"access": "public"
},
"dependencies": {
"@rocket.chat/emitter": "workspace:~",
"react-aria": "~3.37.0"
}
}
91 changes: 67 additions & 24 deletions packages/fuselage-forms/src/Field/FieldContext.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useSafeRefCallback } from '@rocket.chat/fuselage-hooks';
import type { ReactNode, RefCallback } from 'react';
import {
createContext,
Expand All @@ -15,6 +16,8 @@ type FieldContextValue = {
id: string;
fieldType: FieldType;
setFieldType: (fieldType: FieldType) => void;
emitAction: () => void;
onAction: (cb: () => void) => void;
};

export const FieldContext = createContext<FieldContextValue>({
Expand All @@ -25,6 +28,8 @@ export const FieldContext = createContext<FieldContextValue>({
id: '',
fieldType: 'referencedByLabel',
setFieldType: () => {},
emitAction: () => {},
onAction: () => {},
});

export type LabelTypes = 'hint' | 'description' | 'error' | 'placeholder';
Expand All @@ -34,28 +39,43 @@ export type FieldType =
| 'referencedByLabel'
| 'referencedByInput';

const getTextFromNode = (node: HTMLElement) => {
if (!node.textContent) {
return null;
}

const text = [];
const treeWalker = node.ownerDocument.createTreeWalker(
node,
NodeFilter.SHOW_TEXT,
);

while (treeWalker.nextNode()) {
text.push(treeWalker.currentNode.textContent);
}

return text.join(' ');
};

export const useFieldLabel = (): [RefCallback<HTMLElement>, string] => {
const { setLabel, id } = useContext(FieldContext);

const setLabelRef = useCallback(
(node: HTMLElement) => {
if (!node || !node.textContent) {
setLabel(null);
return;
}
const text = [];
const treeWalker = node.ownerDocument.createTreeWalker(
node,
NodeFilter.SHOW_TEXT,
);

while (treeWalker.nextNode()) {
text.push(treeWalker.currentNode.textContent);
}

setLabel(text.join(' '));
},
[setLabel],
const { setLabel, id, emitAction } = useContext(FieldContext);

const setLabelRef = useSafeRefCallback(
useCallback(
(node: HTMLElement) => {
if (!node) {
return;
}
setLabel(getTextFromNode(node));

const onClick = () => emitAction();

node.addEventListener('click', onClick);

return () => node.removeEventListener('click', onClick);
},
[setLabel, emitAction],
),
);

return [setLabelRef, `${id}-label`];
Expand Down Expand Up @@ -139,14 +159,36 @@ export const useFieldReferencedByLabel = () => {
// label is rendered visually hidden inside the inputs wrapper label
export const useFieldWrappedByInputLabel = (): [
ReactNode,
{ 'aria-describedby': string },
{
'aria-describedby': string;
'id': string;
'aria-invalid': 'true' | 'false';
},
RefCallback<HTMLElement>,
] => {
const { id, label, descriptors, setFieldType } = useContext(FieldContext);
const { id, label, descriptors, setFieldType, onAction } =
useContext(FieldContext);

useEffect(() => {
setFieldType('wrappedByLabel');
}, [setFieldType]);

const refCallback = useSafeRefCallback(
useCallback(
(node: HTMLElement) => {
if (!node) {
return;
}

onAction(() => {
node.focus();
node.click();
});
},
[onAction],
),
);

return useMemo(
() => [
label,
Expand All @@ -155,7 +197,8 @@ export const useFieldWrappedByInputLabel = (): [
'id': getInputId(id, descriptors),
...getAriaInvalid(descriptors),
},
refCallback,
],
[label, id, descriptors],
[label, descriptors, id, refCallback],
);
};
15 changes: 15 additions & 0 deletions packages/fuselage-forms/src/Field/FieldProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Emitter } from '@rocket.chat/emitter';
import type { ReactNode } from 'react';
import { useState, useCallback } from 'react';
import { useId } from 'react-aria';
Expand All @@ -12,6 +13,7 @@ function FieldProvider({ children }: FieldProviderProps) {
const [label, setLabel] = useState<ReactNode | null>(null);
const [descriptors, setDescriptors] = useState(new Set<LabelTypes>());
const [fieldType, setFieldType] = useState<FieldType>('referencedByInput');
const [emitter] = useState(() => new Emitter<{ action: void }>());

const setDescriptor = useCallback(
(type: LabelTypes, unregister?: boolean) => {
Expand All @@ -30,9 +32,22 @@ function FieldProvider({ children }: FieldProviderProps) {
[],
);

const emitAction = useCallback(() => {
emitter.emit('action');
}, [emitter]);

const onAction = useCallback(
(cb: () => void) => {
return emitter.on('action', cb);
},
[emitter],
);

return (
<FieldContext.Provider
value={{
emitAction,
onAction,
setDescriptor,
setLabel,
descriptors,
Expand Down
3 changes: 2 additions & 1 deletion packages/fuselage-forms/src/Inputs/withLabelHelpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ function withVisuallyHiddenLabel<TProps>(
Component: ForwardRefExoticComponent<TProps & WithChildrenLabel>,
) {
const WrappedComponent = function (props: TProps) {
const [label, labelProps] = useFieldWrappedByInputLabel();
const [label, labelProps, labelRef] = useFieldWrappedByInputLabel();
return (
<Component
{...props}
{...labelProps}
ref={labelRef}
labelChildren={<VisuallyHidden>{label}</VisuallyHidden>}
/>
);
Expand Down
71 changes: 65 additions & 6 deletions packages/fuselage-forms/src/test.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { composeStories } from '@storybook/react-webpack5';
import { render } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { axe } from 'jest-axe';

import * as _stories from './Field/Field.stories';
Expand All @@ -9,20 +10,41 @@ import * as _stories from './Field/Field.stories';
// This is an issue in the component itself and not with the fields
const { WithSelect: _, ...stories } = _stories;

const testCases = Object.values(composeStories(stories)).map((Story) => [
Story.storyName || 'Story',
Story,
]);
const composedStories = composeStories(stories);

test.each(testCases)(
const {
WithCheckbox,
WithRadioButton,
WithToggleSwitch,
WithTextArea,
...restStories
} = composedStories;

const mapStories = (stories: Partial<typeof composedStories>) =>
Object.values(stories).map((Story: any) => [
Story.storyName || 'Story',
Story,
]);

const allTestCases = mapStories(composedStories);

const onlyInputs = mapStories(restStories);

const wrappedInputs = mapStories({
WithCheckbox,
WithRadioButton,
WithToggleSwitch,
});

test.each(allTestCases)(
`renders %s without crashing`,
async (_storyname, Story) => {
const tree = render(<Story />);
expect(tree.baseElement).toMatchSnapshot();
},
);

test.each(testCases)(
test.each(allTestCases)(
'%s should have no a11y violations',
async (_storyname, Story) => {
const { container } = render(<Story />);
Expand All @@ -31,3 +53,40 @@ test.each(testCases)(
expect(results).toHaveNoViolations();
},
);

test("Clicking WithTextArea's label should focus the textarea", async () => {
const { getByText, container } = render(<WithTextArea />);

const textarea = container.querySelector('textarea');
const label = getByText('Example', { exact: false });
await userEvent.click(label);

expect(textarea).toHaveFocus();
});

test.each(onlyInputs)(
"Clicking %s's label should focus the input",
async (_storyname, Story) => {
const { getByText, container } = render(<Story />);

const input = container.querySelector('input');
const label = getByText('Example', { exact: false });
await userEvent.click(label);

expect(input).toHaveFocus();
},
);

test.each(wrappedInputs)(
"Clicking %s's label should focus the input and mark it as checked",
async (_storyname, Story) => {
const { getByText, container } = render(<Story />);

const input = container.querySelector('input');
const label = getByText('Example', { exact: false, selector: 'span' });
await userEvent.click(label);

expect(input).toHaveFocus();
expect(input?.checked).toBe(true);
},
);
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useMemo } from 'react';

type SafeCallbackRef<T> = (node: T) => (() => void) | undefined;
type SafeCallbackRef<T> = (node: T) => (() => void) | void;

/**
* useSafeRefCallback will call a cleanup function (returned from the passed callback)
Expand Down
3 changes: 3 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5567,6 +5567,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "@rocket.chat/fuselage-forms@workspace:packages/fuselage-forms"
dependencies:
"@rocket.chat/emitter": "workspace:~"
"@rocket.chat/fuselage": "npm:*"
"@rocket.chat/fuselage-tokens": "workspace:~"
"@storybook/addon-docs": "npm:~9.0.18"
Expand All @@ -5575,6 +5576,7 @@ __metadata:
"@storybook/types": "npm:~8.6.14"
"@testing-library/jest-dom": "npm:~6.6.3"
"@testing-library/react": "npm:~16.3.0"
"@testing-library/user-event": "npm:~14.6.1"
"@types/jest": "npm:~29.5.14"
"@types/jest-axe": "npm:~3.5.9"
"@types/react": "npm:~18.3.23"
Expand All @@ -5597,6 +5599,7 @@ __metadata:
webpack: "npm:~5.100.2"
peerDependencies:
"@rocket.chat/fuselage": "*"
"@rocket.chat/fuselage-hooks": "workspace:~"
react: "*"
react-dom: "*"
languageName: unknown
Expand Down
Loading