diff --git a/.changeset/empty-snails-peel.md b/.changeset/empty-snails-peel.md new file mode 100644 index 0000000000..1fde2598eb --- /dev/null +++ b/.changeset/empty-snails-peel.md @@ -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 diff --git a/packages/fuselage-forms/README.md b/packages/fuselage-forms/README.md index 297ea727a8..2ee8760e7f 100644 --- a/packages/fuselage-forms/README.md +++ b/packages/fuselage-forms/README.md @@ -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: diff --git a/packages/fuselage-forms/package.json b/packages/fuselage-forms/package.json index 45e7683388..2b59cc72c9 100644 --- a/packages/fuselage-forms/package.json +++ b/packages/fuselage-forms/package.json @@ -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", @@ -65,6 +66,7 @@ }, "peerDependencies": { "@rocket.chat/fuselage": "*", + "@rocket.chat/fuselage-hooks": "workspace:~", "react": "*", "react-dom": "*" }, @@ -75,6 +77,7 @@ "access": "public" }, "dependencies": { + "@rocket.chat/emitter": "workspace:~", "react-aria": "~3.37.0" } } diff --git a/packages/fuselage-forms/src/Field/FieldContext.ts b/packages/fuselage-forms/src/Field/FieldContext.ts index 82fe89dd6b..14ef425a09 100644 --- a/packages/fuselage-forms/src/Field/FieldContext.ts +++ b/packages/fuselage-forms/src/Field/FieldContext.ts @@ -1,3 +1,4 @@ +import { useSafeRefCallback } from '@rocket.chat/fuselage-hooks'; import type { ReactNode, RefCallback } from 'react'; import { createContext, @@ -15,6 +16,8 @@ type FieldContextValue = { id: string; fieldType: FieldType; setFieldType: (fieldType: FieldType) => void; + emitAction: () => void; + onAction: (cb: () => void) => void; }; export const FieldContext = createContext({ @@ -25,6 +28,8 @@ export const FieldContext = createContext({ id: '', fieldType: 'referencedByLabel', setFieldType: () => {}, + emitAction: () => {}, + onAction: () => {}, }); export type LabelTypes = 'hint' | 'description' | 'error' | 'placeholder'; @@ -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, 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`]; @@ -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, ] => { - 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, @@ -155,7 +197,8 @@ export const useFieldWrappedByInputLabel = (): [ 'id': getInputId(id, descriptors), ...getAriaInvalid(descriptors), }, + refCallback, ], - [label, id, descriptors], + [label, descriptors, id, refCallback], ); }; diff --git a/packages/fuselage-forms/src/Field/FieldProvider.tsx b/packages/fuselage-forms/src/Field/FieldProvider.tsx index 0a7c1cbd26..fbe0e4c623 100644 --- a/packages/fuselage-forms/src/Field/FieldProvider.tsx +++ b/packages/fuselage-forms/src/Field/FieldProvider.tsx @@ -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'; @@ -12,6 +13,7 @@ function FieldProvider({ children }: FieldProviderProps) { const [label, setLabel] = useState(null); const [descriptors, setDescriptors] = useState(new Set()); const [fieldType, setFieldType] = useState('referencedByInput'); + const [emitter] = useState(() => new Emitter<{ action: void }>()); const setDescriptor = useCallback( (type: LabelTypes, unregister?: boolean) => { @@ -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 ( ( Component: ForwardRefExoticComponent, ) { const WrappedComponent = function (props: TProps) { - const [label, labelProps] = useFieldWrappedByInputLabel(); + const [label, labelProps, labelRef] = useFieldWrappedByInputLabel(); return ( {label}} /> ); diff --git a/packages/fuselage-forms/src/test.spec.tsx b/packages/fuselage-forms/src/test.spec.tsx index 8d76e6279b..fbe33a26c9 100644 --- a/packages/fuselage-forms/src/test.spec.tsx +++ b/packages/fuselage-forms/src/test.spec.tsx @@ -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'; @@ -9,12 +10,33 @@ 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) => + 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(); @@ -22,7 +44,7 @@ test.each(testCases)( }, ); -test.each(testCases)( +test.each(allTestCases)( '%s should have no a11y violations', async (_storyname, Story) => { const { container } = render(); @@ -31,3 +53,40 @@ test.each(testCases)( expect(results).toHaveNoViolations(); }, ); + +test("Clicking WithTextArea's label should focus the textarea", async () => { + const { getByText, container } = render(); + + 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(); + + 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(); + + 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); + }, +); diff --git a/packages/fuselage-hooks/src/useSafeRefCallback/useSafeRefCallback.ts b/packages/fuselage-hooks/src/useSafeRefCallback/useSafeRefCallback.ts index e9ad9f47e3..d7ae4b4931 100644 --- a/packages/fuselage-hooks/src/useSafeRefCallback/useSafeRefCallback.ts +++ b/packages/fuselage-hooks/src/useSafeRefCallback/useSafeRefCallback.ts @@ -1,6 +1,6 @@ import { useMemo } from 'react'; -type SafeCallbackRef = (node: T) => (() => void) | undefined; +type SafeCallbackRef = (node: T) => (() => void) | void; /** * useSafeRefCallback will call a cleanup function (returned from the passed callback) diff --git a/yarn.lock b/yarn.lock index 4b2e312ed4..719d73321b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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" @@ -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" @@ -5597,6 +5599,7 @@ __metadata: webpack: "npm:~5.100.2" peerDependencies: "@rocket.chat/fuselage": "*" + "@rocket.chat/fuselage-hooks": "workspace:~" react: "*" react-dom: "*" languageName: unknown