diff --git a/.eslintrc.js b/.eslintrc.js index 0a1552dc81a..6c01bbbcc58 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -10,6 +10,21 @@ module.exports = { ignorePatterns: ['packages/code-connect/**/*'], + settings: { + 'import/parsers': { + [require.resolve('@typescript-eslint/parser')]: [ + '.ts', + '.cts', + '.mts', + '.tsx', + '.js', + '.jsx', + '.mjs', + '.cjs', + ], + }, + }, + rules: { 'gamut/prefer-themed': 'error', 'gamut/no-css-standalone': 'error', @@ -38,6 +53,13 @@ module.exports = { // being applied to subsequent plugin imports/extensions. Wild. files: ['*.tsx', '*.ts'], rules: { + '@typescript-eslint/no-empty-object-type': [ + 'error', + { + allowInterfaces: 'with-single-extends', + allowObjectTypes: 'always', + }, + ], 'no-void': ['error', { allowAsStatement: true }], // These rules could be useful, but we haven't gotten around to enabling them here // See WEB-2 for general tracking. diff --git a/.nx/version-plans/version-plan-1775236316343.md b/.nx/version-plans/version-plan-1775236316343.md new file mode 100644 index 00000000000..084c9eb4a58 --- /dev/null +++ b/.nx/version-plans/version-plan-1775236316343.md @@ -0,0 +1,12 @@ +--- +eslint-plugin-gamut: major +gamut-illustrations: major +gamut-patterns: major +gamut-styles: major +gamut-icons: major +gamut-tests: major +variance: major +gamut: major +--- + +React 18+19 compatibility: workspace on React 19; peers support React 18.3 and 19. Includes eslint-plugin-gamut and core Gamut packages (major bumps per plan). diff --git a/.nx/version-plans/version-plan-1781295720164.md b/.nx/version-plans/version-plan-1781295720164.md new file mode 100644 index 00000000000..04a1e56bb9c --- /dev/null +++ b/.nx/version-plans/version-plan-1781295720164.md @@ -0,0 +1,5 @@ +--- +gamut: minor +--- + +feat(SelectDropdown): add isCreateable prop + remove SearchIcon diff --git a/jest.config.base.ts b/jest.config.base.ts index 2e38f7e7c85..e4e0e604713 100644 --- a/jest.config.base.ts +++ b/jest.config.base.ts @@ -1,6 +1,7 @@ -import type { Config } from 'jest'; import path from 'node:path'; +import type { Config } from 'jest'; + const COVERAGE_PATH_IGNORE_PATTERNS = [ '/node_modules/', '/dist/', diff --git a/package.json b/package.json index c09a7f053d9..7fe3a3dc24f 100644 --- a/package.json +++ b/package.json @@ -8,9 +8,9 @@ "@emotion/styled": "11.14.1", "@vidstack/react": "^1.12.12", "core-js": "3.7.0", - "lodash": "^4.18.1", - "react": "18.3.1", - "react-dom": "18.3.1", + "lodash": "^4.17.23", + "react": "^19.0.0", + "react-dom": "^19.0.0", "react-helmet-async": "^2.0.5" }, "devDependencies": { @@ -19,7 +19,7 @@ "@babel/preset-env": "^7.24.7", "@babel/preset-react": "^7.24.7", "@babel/preset-typescript": "^7.24.7", - "@codecademy/eslint-config": "8.0.0", + "@codecademy/eslint-config": "8.2.0", "@codecademy/gamut": "workspace:*", "@codecademy/prettier-config": "^0.2.0", "@codecademy/tsconfig": "^0.3.0", @@ -43,27 +43,27 @@ "@svgr/cli": "5.5.0", "@swc-node/register": "^1.11.1", "@swc/core": "^1.15.18", - "@testing-library/dom": "^8.11.1", + "@testing-library/dom": "^10.0.0", "@testing-library/jest-dom": "^5.16.1", - "@testing-library/react": "15.0.6", + "@testing-library/react": "^16.0.0", "@testing-library/react-hooks": "^7.0.2", "@testing-library/user-event": "^14.5.2", "@types/classnames": "2.2.10", "@types/invariant": "2.2.29", "@types/konami-code-js": "^0.8.0", "@types/lodash": "4.17.23", - "@types/react": "18.3.27", - "@types/react-dom": "18.3.1", - "@types/react-test-renderer": "18.3.0", + "@types/react": "^18.2.0", + "@types/react-dom": "^18.2.0", + "@types/react-test-renderer": "^18.2.0", "@types/stylis": "^4.2.0", - "@typescript-eslint/eslint-plugin": "^5.15.0", - "@typescript-eslint/parser": "^5.15.0", + "@typescript-eslint/eslint-plugin": "^8.57.0", + "@typescript-eslint/parser": "^8.57.0", "babel-jest": "29.6.4", - "babel-plugin-macros": "3.0.1", + "babel-plugin-macros": "^3.0.1", "commander": "^14.0.3", "component-test-setup": "^0.3.1", "cpy-cli": "^4.1.0", - "eslint": "^8.11.0", + "eslint": "^8.57.0", "eslint-plugin-gamut": "workspace:*", "eslint-plugin-local-rules": "^1.1.0", "eslint-plugin-lodash": "^7.4.0", @@ -82,7 +82,7 @@ "onchange": "^7.0.2", "playwright": "^1.49.1", "prettier": "^2.8.7", - "react-test-renderer": "18.3.1", + "react-test-renderer": "^19.0.0", "start-server-and-test": "^2.1.5", "storybook": "^10.3.1", "storybook-addon-deep-controls": "^0.10.0", @@ -113,13 +113,14 @@ "repository": "git@github.com:Codecademy/gamut.git", "resolutions": { "@react-aria/interactions": "3.25.0", - "@types/react": "18.3.27", - "@types/react-dom": "18.3.1", - "@typescript-eslint/utils": "^5.15.0", + "@types/react": "^18.2.0", + "@types/react-dom": "^18.2.0", + "@types/react-test-renderer": "^18.2.0", + "@typescript-eslint/utils": "^8.57.0", "axios": "1.14.0", "error-ex": "1.3.4", - "react": "18.3.1", - "react-dom": "18.3.1" + "react": "^19.0.0", + "react-dom": "^19.0.0" }, "scripts": { "build": "nx run-many --target=build --all", diff --git a/packages/eslint-plugin-gamut/package.json b/packages/eslint-plugin-gamut/package.json index deafc93628f..9a3b24d5c0a 100644 --- a/packages/eslint-plugin-gamut/package.json +++ b/packages/eslint-plugin-gamut/package.json @@ -4,7 +4,7 @@ "version": "2.4.3", "author": "Codecademy Engineering ", "dependencies": { - "@typescript-eslint/utils": "^5.15.0" + "@typescript-eslint/utils": "^8.57.0" }, "files": [ "dist" diff --git a/packages/eslint-plugin-gamut/src/gamut-import-paths.ts b/packages/eslint-plugin-gamut/src/gamut-import-paths.ts index a7fcb9601fb..e2253ba7b64 100644 --- a/packages/eslint-plugin-gamut/src/gamut-import-paths.ts +++ b/packages/eslint-plugin-gamut/src/gamut-import-paths.ts @@ -59,7 +59,6 @@ export default createRule({ meta: { docs: { description: 'Ensure Gamut import statements have proper module paths.', - recommended: 'error', }, fixable: 'code', messages: { diff --git a/packages/eslint-plugin-gamut/src/no-css-standalone.test.ts b/packages/eslint-plugin-gamut/src/no-css-standalone.test.ts index cada3987044..1094f1e4993 100644 --- a/packages/eslint-plugin-gamut/src/no-css-standalone.test.ts +++ b/packages/eslint-plugin-gamut/src/no-css-standalone.test.ts @@ -1,9 +1,9 @@ -import { ESLintUtils } from '@typescript-eslint/utils'; +import { TSESLint } from '@typescript-eslint/utils'; import rule from './no-css-standalone'; -const ruleTester = new ESLintUtils.RuleTester({ - parser: '@typescript-eslint/parser', +const ruleTester = new TSESLint.RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), }); ruleTester.run('no-css-standalone', rule, { diff --git a/packages/eslint-plugin-gamut/src/no-css-standalone.ts b/packages/eslint-plugin-gamut/src/no-css-standalone.ts index fe1c66a8656..e6e2e55fd2f 100644 --- a/packages/eslint-plugin-gamut/src/no-css-standalone.ts +++ b/packages/eslint-plugin-gamut/src/no-css-standalone.ts @@ -15,7 +15,6 @@ export default createRule({ meta: { docs: { description: 'Ensure no standalone .css or .scss files.', - recommended: 'error', }, messages: { noCssStandalone: diff --git a/packages/eslint-plugin-gamut/src/no-inline-style.test.ts b/packages/eslint-plugin-gamut/src/no-inline-style.test.ts index a1c24e75981..ab7c5c53e13 100644 --- a/packages/eslint-plugin-gamut/src/no-inline-style.test.ts +++ b/packages/eslint-plugin-gamut/src/no-inline-style.test.ts @@ -1,9 +1,9 @@ -import { ESLintUtils } from '@typescript-eslint/utils'; +import { TSESLint } from '@typescript-eslint/utils'; import rule from './no-inline-style'; -const ruleTester = new ESLintUtils.RuleTester({ - parser: '@typescript-eslint/parser', +const ruleTester = new TSESLint.RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), parserOptions: { ecmaFeatures: { jsx: true, diff --git a/packages/eslint-plugin-gamut/src/no-inline-style.ts b/packages/eslint-plugin-gamut/src/no-inline-style.ts index 8e063bbde13..df899dd38b4 100644 --- a/packages/eslint-plugin-gamut/src/no-inline-style.ts +++ b/packages/eslint-plugin-gamut/src/no-inline-style.ts @@ -1,10 +1,15 @@ +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + import { createRule } from './createRule'; export default createRule({ create(context) { return { JSXAttribute(node) { - if (node.name.type === 'JSXIdentifier' && node.name.name === 'style') { + if ( + node.name.type === AST_NODE_TYPES.JSXIdentifier && + node.name.name === 'style' + ) { context.report({ messageId: 'noInlineStyle', node, @@ -17,7 +22,6 @@ export default createRule({ meta: { docs: { description: 'Disallow inline style props on JSX elements.', - recommended: 'error', }, messages: { noInlineStyle: diff --git a/packages/eslint-plugin-gamut/src/no-kbd-element.test.ts b/packages/eslint-plugin-gamut/src/no-kbd-element.test.ts index fc2c7f13da1..30dee531773 100644 --- a/packages/eslint-plugin-gamut/src/no-kbd-element.test.ts +++ b/packages/eslint-plugin-gamut/src/no-kbd-element.test.ts @@ -1,9 +1,9 @@ -import { ESLintUtils } from '@typescript-eslint/utils'; +import { TSESLint } from '@typescript-eslint/utils'; import rule from './no-kbd-element'; -const ruleTester = new ESLintUtils.RuleTester({ - parser: '@typescript-eslint/parser', +const ruleTester = new TSESLint.RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), parserOptions: { ecmaFeatures: { jsx: true, diff --git a/packages/eslint-plugin-gamut/src/no-kbd-element.ts b/packages/eslint-plugin-gamut/src/no-kbd-element.ts index f90de6d1cbc..1e644060933 100644 --- a/packages/eslint-plugin-gamut/src/no-kbd-element.ts +++ b/packages/eslint-plugin-gamut/src/no-kbd-element.ts @@ -1,10 +1,15 @@ +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + import { createRule } from './createRule'; export default createRule({ create(context) { return { JSXOpeningElement(node) { - if (node.name.type === 'JSXIdentifier' && node.name.name === 'kbd') { + if ( + node.name.type === AST_NODE_TYPES.JSXIdentifier && + node.name.name === 'kbd' + ) { context.report({ messageId: 'noKbdElement', node, @@ -18,7 +23,6 @@ export default createRule({ docs: { description: 'Intended to be used in Storybook docs to disallow use of the `kbd` HTML element in favor of the `KeyboardKey` component for styling purposes.', - recommended: 'error', }, messages: { noKbdElement: 'Please use the `KeyboardKey` component instead.', diff --git a/packages/eslint-plugin-gamut/src/prefer-themed.test.ts b/packages/eslint-plugin-gamut/src/prefer-themed.test.ts index 1a438cd3247..3d9383afb0c 100644 --- a/packages/eslint-plugin-gamut/src/prefer-themed.test.ts +++ b/packages/eslint-plugin-gamut/src/prefer-themed.test.ts @@ -1,9 +1,9 @@ -import { ESLintUtils } from '@typescript-eslint/utils'; +import { TSESLint } from '@typescript-eslint/utils'; import rule from './prefer-themed'; -const ruleTester = new ESLintUtils.RuleTester({ - parser: '@typescript-eslint/parser', +const ruleTester = new TSESLint.RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), }); ruleTester.run('prefer-themed', rule, { diff --git a/packages/eslint-plugin-gamut/src/prefer-themed.ts b/packages/eslint-plugin-gamut/src/prefer-themed.ts index 45892f197e5..ebeca764c31 100644 --- a/packages/eslint-plugin-gamut/src/prefer-themed.ts +++ b/packages/eslint-plugin-gamut/src/prefer-themed.ts @@ -1,5 +1,3 @@ -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; - import { createRule } from './createRule'; import { checkArrowFuncBodyTypesAndReturnThemeVars, @@ -10,15 +8,14 @@ export default createRule({ create(context) { return { TaggedTemplateExpression(node) { - if (node.tag.type === AST_NODE_TYPES.MemberExpression) { + /* eslint-disable @typescript-eslint/no-unsafe-enum-comparison -- @typescript-eslint/types use string enums; literals match ESTree */ + if (node.tag.type === 'MemberExpression') { if (node.tag.object.type !== 'Identifier') return; const expressionVariable = node.tag.object.name; const arrowFuncExpression = node.quasi.expressions[0]; - if ( - arrowFuncExpression?.type !== AST_NODE_TYPES.ArrowFunctionExpression - ) - return; + if (arrowFuncExpression?.type !== 'ArrowFunctionExpression') return; + /* eslint-enable @typescript-eslint/no-unsafe-enum-comparison */ if (!isNamedVariableTheme(arrowFuncExpression)) return; @@ -48,7 +45,6 @@ export default createRule({ meta: { docs: { description: 'Prefer themed style utility', - recommended: 'error', }, fixable: 'code', messages: { diff --git a/packages/gamut-icons/package.json b/packages/gamut-icons/package.json index 1de78c04e13..e59c2b6904d 100644 --- a/packages/gamut-icons/package.json +++ b/packages/gamut-icons/package.json @@ -17,7 +17,7 @@ "@emotion/react": "^11.4.0", "@emotion/styled": "^11.3.0", "lodash": "^4.17.23", - "react": "^17.0.2 || ^18.3.0" + "react": "^17.0.2 || ^18.3.0 || ^19.0.0" }, "publishConfig": { "access": "public" diff --git a/packages/gamut-illustrations/package.json b/packages/gamut-illustrations/package.json index e0918dfda97..b253fc6c475 100644 --- a/packages/gamut-illustrations/package.json +++ b/packages/gamut-illustrations/package.json @@ -18,8 +18,8 @@ "peerDependencies": { "@emotion/react": "^11.4.0", "@emotion/styled": "^11.3.0", - "react": "^17.0.2 || ^18.3.0", - "react-dom": "^17.0.2 || ^18.3.0" + "react": "^17.0.2 || ^18.3.0 || ^19.0.0", + "react-dom": "^17.0.2 || ^18.3.0 || ^19.0.0" }, "publishConfig": { "access": "public" diff --git a/packages/gamut-patterns/package.json b/packages/gamut-patterns/package.json index bc2ffbd48f9..afc1d419946 100644 --- a/packages/gamut-patterns/package.json +++ b/packages/gamut-patterns/package.json @@ -19,8 +19,8 @@ "peerDependencies": { "@emotion/react": "^11.4.0", "@emotion/styled": "^11.3.0", - "react": "^17.0.2 || ^18.3.0", - "react-dom": "^17.0.2 || ^18.3.0" + "react": "^17.0.2 || ^18.3.0 || ^19.0.0", + "react-dom": "^17.0.2 || ^18.3.0 || ^19.0.0" }, "publishConfig": { "access": "public" diff --git a/packages/gamut-styles/package.json b/packages/gamut-styles/package.json index e852020bf2b..e6db76629c7 100644 --- a/packages/gamut-styles/package.json +++ b/packages/gamut-styles/package.json @@ -6,7 +6,7 @@ "dependencies": { "@codecademy/variance": "0.26.1", "@emotion/is-prop-valid": "^1.1.0", - "framer-motion": "^11.18.0", + "framer-motion": "^12.0.0", "get-nonce": "^1.0.0", "polished": "^4.1.2" }, @@ -26,7 +26,7 @@ "@emotion/react": "^11.4.0", "@emotion/styled": "^11.3.0", "lodash": "^4.17.23", - "react": "^17.0.2 || ^18.3.0", + "react": "^17.0.2 || ^18.3.0 || ^19.0.0", "stylis": "^4.0.7" }, "publishConfig": { diff --git a/packages/gamut-styles/src/__tests__/AssetProvider.test.tsx b/packages/gamut-styles/src/__tests__/AssetProvider.test.tsx index 7a38ef6fa61..15fc801e839 100644 --- a/packages/gamut-styles/src/__tests__/AssetProvider.test.tsx +++ b/packages/gamut-styles/src/__tests__/AssetProvider.test.tsx @@ -4,12 +4,13 @@ import { render } from '@testing-library/react'; import { AssetProvider, createFontLinks } from '../AssetProvider'; import { coreTheme, percipioTheme } from '../themes'; +import { getFontsMock } from './fontUtilsMock'; import { setupRtl } from './testUtils'; const renderView = setupRtl(AssetProvider, {}); jest.mock('../utilities/fontUtils', () => ({ - getFonts: jest.fn(), + getFonts: require('./fontUtilsMock').getFontsMock, })); jest.mock('../remoteAssets/fonts', () => { @@ -52,11 +53,36 @@ jest.mock('../remoteAssets/fonts', () => { }; }); -const mockGetFonts = require('../utilities/fontUtils').getFonts; +const mockGetFonts = getFontsMock; + +function getPreloadLinks(container: HTMLElement): NodeListOf { + const inContainer = container.querySelectorAll('link[rel="preload"]'); + if (inContainer.length > 0) return inContainer; + return document.querySelectorAll('link[rel="preload"]'); +} + +const defaultFonts = [ + { + filePath: 'https://www.codecademy.com/gamut/apercu-regular-pro', + extensions: ['woff2', 'woff'], + name: 'Apercu', + }, + { + filePath: 'https://www.codecademy.com/gamut/apercu-bold-pro', + extensions: ['woff2', 'woff'], + name: 'Apercu', + weight: 'bold', + }, +]; describe('AssetProvider', () => { beforeEach(() => { jest.clearAllMocks(); + mockGetFonts.mockReturnValue(defaultFonts); + }); + + afterEach(() => { + mockGetFonts.mockReturnValue(defaultFonts); }); describe('createFontLinks', () => { @@ -75,7 +101,7 @@ describe('AssetProvider', () => { ]; const { container } = render(<>{createFontLinks(fonts)}); - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(1); expect(links[0]).toHaveAttribute( @@ -89,13 +115,13 @@ describe('AssetProvider', () => { it('should handle empty fonts array', () => { const { container } = render(<>{createFontLinks([])}); - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(0); }); it('should handle undefined fonts parameter', () => { const { container } = render(<>{createFontLinks(undefined)}); - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(2); }); @@ -119,7 +145,7 @@ describe('AssetProvider', () => { ]; const { container } = render(<>{createFontLinks(fonts)}); - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(1); expect(links[0]).toHaveAttribute( 'href', @@ -148,7 +174,7 @@ describe('AssetProvider', () => { ]; const { container } = render(<>{createFontLinks(fonts)}); - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(2); }); }); @@ -164,14 +190,14 @@ describe('AssetProvider', () => { ]); const { view } = renderView(); - const links = view.container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(view.container); + expect(mockGetFonts).toHaveBeenCalledWith('core'); expect(links).toHaveLength(1); expect(links[0]).toHaveAttribute( 'href', 'https://www.codecademy.com/gamut/apercu-regular-pro.woff2' ); - expect(mockGetFonts).toHaveBeenCalledWith('core'); }); it('should render font links for percipio theme', () => { @@ -184,7 +210,7 @@ describe('AssetProvider', () => { ]); const { view } = renderView({ theme: percipioTheme as any }); - const links = view.container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(view.container); expect(links).toHaveLength(1); expect(links[0]).toHaveAttribute( @@ -216,7 +242,7 @@ describe('AssetProvider', () => { }); const { view } = renderView(); - const links = view.container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(view.container); expect(links).toHaveLength(0); }); @@ -224,7 +250,7 @@ describe('AssetProvider', () => { mockGetFonts.mockReturnValue(undefined); const { view } = renderView(); - const links = view.container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(view.container); expect(links).toHaveLength(2); }); @@ -232,7 +258,7 @@ describe('AssetProvider', () => { mockGetFonts.mockReturnValue(null); const { view } = renderView(); - const links = view.container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(view.container); expect(links).toHaveLength(2); }); @@ -240,7 +266,7 @@ describe('AssetProvider', () => { mockGetFonts.mockReturnValue('not-an-array'); const { view } = renderView(); - const links = view.container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(view.container); expect(links).toHaveLength(0); }); @@ -264,7 +290,7 @@ describe('AssetProvider', () => { ]); const { view } = renderView(); - const links = view.container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(view.container); expect(links).toHaveLength(3); expect(links[0]).toHaveAttribute( @@ -301,7 +327,7 @@ describe('AssetProvider', () => { ]); const { view } = renderView(); - const links = view.container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(view.container); expect(links).toHaveLength(1); expect(links[0]).toHaveAttribute( @@ -330,7 +356,7 @@ describe('AssetProvider', () => { ]); const { view } = renderView(); - const links = view.container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(view.container); expect(links).toHaveLength(1); expect(links[0]).toHaveAttribute( diff --git a/packages/gamut-styles/src/__tests__/fontLoading.test.tsx b/packages/gamut-styles/src/__tests__/fontLoading.test.tsx index 00bd1026b39..b96acc16a96 100644 --- a/packages/gamut-styles/src/__tests__/fontLoading.test.tsx +++ b/packages/gamut-styles/src/__tests__/fontLoading.test.tsx @@ -2,12 +2,12 @@ import { render } from '@testing-library/react'; import { AssetProvider } from '../AssetProvider'; import { coreTheme, percipioTheme } from '../themes'; +import { getFontsMock } from './fontUtilsMock'; -// Type assertion to satisfy Theme interface in GamutProvider from theme.d.ts - this lib is typed to the CoreTheme interface const typedPercipioTheme = percipioTheme as any; jest.mock('../utilities/fontUtils', () => ({ - getFonts: jest.fn(), + getFonts: require('./fontUtilsMock').getFontsMock, })); jest.mock('../remoteAssets/fonts', () => { @@ -38,7 +38,13 @@ jest.mock('../remoteAssets/fonts', () => { }; }); -const mockGetFonts = require('../utilities/fontUtils').getFonts; +const mockGetFonts = getFontsMock; + +function getPreloadLinks(container: HTMLElement): NodeListOf { + const inContainer = container.querySelectorAll('link[rel="preload"]'); + if (inContainer.length > 0) return inContainer; + return document.querySelectorAll('link[rel="preload"]'); +} const mockDocumentFonts = { load: jest.fn(), @@ -56,14 +62,27 @@ Object.defineProperty(document, 'fonts', { const mockFetch = jest.fn(); global.fetch = mockFetch; +const defaultFonts = [ + { + filePath: 'https://www.codecademy.com/gamut/apercu-regular-pro', + extensions: ['woff2', 'woff'], + name: 'Apercu', + }, +]; + describe('Font Loading and Error Handling', () => { beforeEach(() => { jest.clearAllMocks(); + mockGetFonts.mockReturnValue(defaultFonts); mockDocumentFonts.load.mockClear(); mockDocumentFonts.check.mockClear(); mockFetch.mockClear(); }); + afterEach(() => { + mockGetFonts.mockReturnValue(defaultFonts); + }); + describe('Font Preloading', () => { it('should create preload links for fonts', () => { mockGetFonts.mockReturnValue([ @@ -76,7 +95,7 @@ describe('Font Loading and Error Handling', () => { const { container } = render(); - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(1); expect(links[0]).toHaveAttribute( 'href', @@ -102,7 +121,7 @@ describe('Font Loading and Error Handling', () => { const { container } = render(); - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(2); expect(links[0]).toHaveAttribute( 'href', @@ -123,8 +142,7 @@ describe('Font Loading and Error Handling', () => { const { container } = render(); - // Should not render any links when getFonts fails - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(0); }); @@ -150,7 +168,7 @@ describe('Font Loading and Error Handling', () => { const { container } = render(); - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(2); }); }); @@ -172,8 +190,7 @@ describe('Font Loading and Error Handling', () => { const { container } = render(); - // Should render preload links for all fonts - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(2); expect(links[0]).toHaveAttribute( 'href', @@ -204,7 +221,7 @@ describe('Font Loading and Error Handling', () => { const { container } = render(); - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(1); Object.defineProperty(document, 'fonts', { @@ -227,7 +244,7 @@ describe('Font Loading and Error Handling', () => { const { container } = render(); - const links = container.querySelectorAll('link[rel="preload"]'); + const links = getPreloadLinks(container); expect(links).toHaveLength(1); global.fetch = originalFetch; diff --git a/packages/gamut-styles/src/__tests__/fontUtilsMock.ts b/packages/gamut-styles/src/__tests__/fontUtilsMock.ts new file mode 100644 index 00000000000..22501405fc9 --- /dev/null +++ b/packages/gamut-styles/src/__tests__/fontUtilsMock.ts @@ -0,0 +1 @@ +export const getFontsMock = jest.fn(); diff --git a/packages/gamut-styles/src/variance/utils.ts b/packages/gamut-styles/src/variance/utils.ts index a88b1bfb116..efccefdd8cd 100644 --- a/packages/gamut-styles/src/variance/utils.ts +++ b/packages/gamut-styles/src/variance/utils.ts @@ -1,5 +1,6 @@ import { ThemeProps } from '@codecademy/variance'; import isPropValid from '@emotion/is-prop-valid'; +import type React from 'react'; import { all as allProps } from './config'; @@ -17,10 +18,10 @@ const validPropnames = allPropnames.filter(isPropValid); export type SystemPropNames = (typeof allPropnames)[number]; -export type ElementOrProps = keyof JSX.IntrinsicElements | ThemeProps; +export type ElementOrProps = keyof React.JSX.IntrinsicElements | ThemeProps; export type ForwardableProps = Exclude< - El extends keyof JSX.IntrinsicElements - ? keyof JSX.IntrinsicElements[El] + El extends keyof React.JSX.IntrinsicElements + ? keyof React.JSX.IntrinsicElements[El] : keyof Element, Additional | SystemPropNames >; diff --git a/packages/gamut-styles/tsconfig.lib.json b/packages/gamut-styles/tsconfig.lib.json index bf001194960..daaeddbd0e7 100644 --- a/packages/gamut-styles/tsconfig.lib.json +++ b/packages/gamut-styles/tsconfig.lib.json @@ -12,6 +12,7 @@ ], "exclude": [ "jest.config.ts", + "**/__tests__/**", "**/*.spec.ts", "**/*.test.ts", "**/*.spec.tsx", diff --git a/packages/gamut-tests/package.json b/packages/gamut-tests/package.json index 4e72c1b036a..d82c07252e1 100644 --- a/packages/gamut-tests/package.json +++ b/packages/gamut-tests/package.json @@ -5,7 +5,7 @@ "author": "Codecademy Engineering ", "dependencies": { "@codecademy/gamut-styles": "20.0.2", - "component-test-setup": "*", + "component-test-setup": "^0.3.1", "lodash": "^4.17.23" }, "files": [ @@ -22,7 +22,7 @@ "main": "dist/index.js", "module": "dist/index.js", "peerDependencies": { - "react": "^17.0.2 || ^18.3.0" + "react": "^17.0.2 || ^18.3.0 || ^19.0.0" }, "publishConfig": { "access": "public" diff --git a/packages/gamut-tests/src/index.tsx b/packages/gamut-tests/src/index.tsx index 5d63b51587e..8d9c73cadf2 100644 --- a/packages/gamut-tests/src/index.tsx +++ b/packages/gamut-tests/src/index.tsx @@ -21,7 +21,7 @@ export const MockGamutProvider: React.FC<{ ); }; -function withMockGamutProvider( +function withMockGamutProvider( WrappedComponent: React.ComponentType ) { const WithBoundaryComponent: React.FC = (props) => ( diff --git a/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md b/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md index c135ed355c7..6ba8f776698 100644 --- a/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md +++ b/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md @@ -26,6 +26,22 @@ For typical product forms, prefer `GridForm` (declarative `fields`, `LayoutGrid` --- +## SelectDropdown vs Select + +Use `Select` for standard single-select fields where bundle size matters and no special styling is needed. Use `SelectDropdown` when the design calls for any of: + +- Styled dropdown menu (react-select appearance) +- Search / typeahead +- Multi-select with tags +- Creatable options +- Option icons, subtitles, right labels, abbreviations, or grouped options + +`SelectDropdown` carries a larger JS dependency (react-select); don't reach for it as a default drop-in for `Select`. + +For full SelectDropdown API detail — controlled vs uncontrolled patterns, creatable options, react-select action metadata — use [`gamut-select-dropdown`](../gamut-select-dropdown/SKILL.md). Generic `FormGroup` wiring (labels, errors, live regions) still applies as documented below. + +--- + ## `FormGroup` (baseline) `packages/gamut/src/Form/elements/FormGroup.tsx` diff --git a/packages/gamut/agent-tools/skills/gamut-select-dropdown/SKILL.md b/packages/gamut/agent-tools/skills/gamut-select-dropdown/SKILL.md new file mode 100644 index 00000000000..49285e1b5f7 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-select-dropdown/SKILL.md @@ -0,0 +1,236 @@ +--- +name: gamut-select-dropdown +description: Use when implementing or auditing SelectDropdown — single/multi modes, controlled vs uncontrolled value, creatable options, FormGroup wiring, and onChange contract. Pair with gamut-forms for error live regions, ConnectedForm, and field-level validation. +--- + +# Gamut SelectDropdown + +Styled dropdown built on react-select. + +Source: `@codecademy/gamut` — [SelectDropdown.tsx](https://github.com/Codecademy/gamut/blob/main/packages/gamut/src/Form/SelectDropdown/SelectDropdown.tsx) + +See also: [`gamut-forms`](../gamut-forms/SKILL.md) — FormGroup wiring, error regions, and validation UX. + +Storybook: [Atoms / FormInputs / SelectDropdown](https://gamut.codecademy.com/?path=/docs-atoms-forminputs-selectdropdown--docs) + +--- + +## When to use SelectDropdown vs Select + +Use `Select` for standard single-select forms with minimal bundle cost. Use `SelectDropdown` when designs specify the styled dropdown menu, search, multi-select tags, creatable options, icons, groups, or abbreviations. SelectDropdown has a larger JavaScript dependency (react-select). + +--- + +## Options + +`options` accepts plain strings or option objects. `value` is always a string and references an option's `value`. + +| Field | Required | Notes | +| -------------- | -------- | -------------------------------------------------------------------- | +| `label` | yes | Display text | +| `value` | yes | Unique string; what `value` / `string[]` reference | +| `disabled` | no | Option cannot be selected | +| `subtitle` | no | Secondary text below the label | +| `rightLabel` | no | Text on the right side of the option | +| `icon` | no | A `@codecademy/gamut-icons` component | +| `abbreviation` | no | Short text shown in the input while the full label shows in the menu | + +Grouped options: `{ label, options: [...], divider? }` (extends react-select `GroupBase`; `divider` draws a rule above the group). + +--- + +## Controlled vs uncontrolled + +SelectDropdown does **not** accept `defaultValue`. + +| Mode | Uncontrolled | Controlled | +| ---------------- | -------------------------------------------------- | --------------------------------------------------------------------------------- | +| Single | Not supported | `value` (string) + update in `onChange` | +| Multi | Omit `value` or pass non-array (`undefined`, `''`) | `value: string[]` + update in `onChange` | +| Creatable single | Not supported | Same as single; `onCreateOption` appends to `options` | +| Creatable multi | Omit `value`; `onCreateOption` for options | `value: string[]`; update in `onChange` on every change including `create-option` | + +Single-select selection is derived from the `value` prop only — internal state is not kept. Multi-select without `value: string[]` keeps selection in internal `multiValues`. + +**Controlled creatable multi pitfall:** Updating `options` alone without syncing `value` in `onChange` clears selection when options re-render. + +### When to use uncontrolled (multi only) + +Uncontrolled multi is appropriate when: + +- No other part of the UI needs to react to the current selection (no live summary, no dependent field, no enabled/disabled button). +- You only need the value at form submission — via `FormData`, a submit handler reading the DOM, or react-hook-form's `getValues`. +- Simplicity is the priority; omitting `value` means one less piece of state to manage. + +```tsx +// Good fit: a "tags" field where only the submitted array matters + setTagOptions((prev) => [...prev, v])} +/> +``` + +### When to use controlled + +Use controlled when: + +- Another part of the UI must reflect the current selection in real time (summary text, a filtered list, an enable/disable condition). +- You need to pre-populate from an API response, reset on cancel, or sync with a form library like react-hook-form. +- You are using single-select (the only supported mode for single). + +```tsx +// Good fit: pre-populate from API, clear on cancel, show live summary +const [selected, setSelected] = useState(initialValues); + + setSelected(opts.map((o) => o.value))} +/> +

Selected: {selected.join(', ') || 'none'}

+``` + +--- + +## onChange contract + +`onChange` receives option object(s), not `event.target.value`: + +```tsx +// Single +onChange={(option) => setValue(option.value)} + +// Multi +onChange={(selected) => setValue(selected.map((o) => o.value))} +``` + +Second argument is react-select `ActionMeta`. For creatable creates: `meta.action === 'create-option'`. Do **not** pass `onCreateOption` to react-select directly — Gamut invokes it from `changeHandler` while still forwarding `create-option` to consumer `onChange`. + +--- + +## Creatable + +- `isCreatable` forces `isSearchable: true` (TypeScript enforces this). +- `onCreateOption(inputValue)` — convenience hook to append to `options`. +- `onChange(selected, meta)` — use `meta.action === 'create-option'` to sync controlled `value` and `options` together. +- `isValidNewOption` — return `false` to hide the Add row. +- `validationMessage` — replaces menu "No options" text; mirror in `FormGroup` `error` for field-level feedback. + +**Validation after blur:** react-select clears input on blur before `onBlur` fires, so the value is gone by the time you'd validate it. Store the last typed value in a ref and re-validate from it on `input-blur`: + +```tsx +const lastInput = useRef(''); + + { + if (action === 'input-change') lastInput.current = value; + if (action === 'input-blur') validate(lastInput.current); + }} +/>; +``` + +--- + +## FormGroup wiring + +- `FormGroup` `htmlFor` must match control `id` (not `name`). Alternatively, pass `htmlFor` directly on SelectDropdown and it becomes `id` downstream. +- Pass `name` on SelectDropdown (required for forms). +- Pass `aria-label` (required for forms); it must match the FormGroupLabel `htmlFor`. +- Pass `error` boolean when FormGroup has an error. +- Generic FormGroup live-region behavior: see [`gamut-forms`](../gamut-forms/SKILL.md). + +```tsx + + setValue(option.value)} + /> + +``` + +--- + +## Styling & layout props + +| Prop | Type | Default | Notes | +| ------------------- | ------------------------ | -------- | --------------------------------------------------------- | +| `size` | `'small' \| 'medium'` | `medium` | Control height/density | +| `shownOptionsLimit` | `1`–`6` | `6` | Visible options before the menu scrolls | +| `inputWidth` | `string \| number` | — | Width of the input independent of the menu | +| `dropdownWidth` | `string \| number` | — | Width of the menu independent of the input | +| `menuAlignment` | `'left' \| 'right'` | `left` | Menu edge alignment | +| `zIndex` | `number` | auto | Menu z-index | +| `inputProps` | `{ hidden?, combobox? }` | — | `data-*` / `aria-*` only, forwarded to the input elements | + +--- + +## Examples + +### Single (controlled) + +```tsx +const [value, setValue] = useState('us'); + + setValue(option.value)} +/>; +``` + +### Multi (uncontrolled) + +```tsx + console.log(selected)} +/> +``` + +### Creatable multi (uncontrolled) + +```tsx +const [options, setOptions] = useState(['Apple', 'Banana']); + + setOptions((prev) => [...prev, v])} +/>; +``` + +### Creatable multi (controlled) + +```tsx +const [options, setOptions] = useState(['Apple', 'Banana']); +const [value, setValue] = useState([]); + + { + setValue(selected.map((o) => o.value)); + if (meta.action === 'create-option' && meta.option) { + setOptions((prev) => [...prev, meta.option.value]); + } + }} +/>; +``` diff --git a/packages/gamut/package.json b/packages/gamut/package.json index 1f4a2fc1bc1..1291d9e5480 100644 --- a/packages/gamut/package.json +++ b/packages/gamut/package.json @@ -15,7 +15,7 @@ "@types/marked": "^4.0.8", "@vidstack/react": "^1.12.12", "classnames": "^2.2.5", - "framer-motion": "^11.18.0", + "framer-motion": "^12.0.0", "html-to-react": "^1.6.0", "invariant": "^2.2.4", "lodash": "^4.17.23", @@ -23,7 +23,7 @@ "polished": "^4.1.2", "react-aria-components": "1.7.1", "react-focus-on": "3.10.0", - "react-hook-form": "^7.65.0", + "react-hook-form": "^7.71.2", "react-player": "^2.16.0", "react-select": "^5.2.2", "react-truncate-markup": "^5.1.2", @@ -44,8 +44,8 @@ "peerDependencies": { "@emotion/react": "^11.4.0", "@emotion/styled": "^11.3.0", - "react": "^17.0.2 || ^18.3.0", - "react-dom": "^17.0.2 || ^18.3.0" + "react": "^17.0.2 || ^18.3.0 || ^19.0.0", + "react-dom": "^17.0.2 || ^18.3.0 || ^19.0.0" }, "publishConfig": { "access": "public" diff --git a/packages/gamut/src/Anchor/__tests__/Anchor.test.tsx b/packages/gamut/src/Anchor/__tests__/Anchor.test.tsx index 7aedaca8f59..5e36e529d60 100644 --- a/packages/gamut/src/Anchor/__tests__/Anchor.test.tsx +++ b/packages/gamut/src/Anchor/__tests__/Anchor.test.tsx @@ -1,6 +1,7 @@ import { MiniWarningTriangleIcon } from '@codecademy/gamut-icons'; import { setupRtl } from '@codecademy/gamut-tests'; -import { screen } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; +import React from 'react'; import { Anchor } from '..'; @@ -33,4 +34,21 @@ describe('Anchor', () => { expect(buttonElement).toHaveTextContent(anchorText); }); + + it('forwards ref to the anchor element', () => { + const ref = React.createRef(); + render( + + {anchorText} + + ); + expect(ref.current).toBeInstanceOf(HTMLAnchorElement); + expect(ref.current).toHaveAttribute('href', href); + }); + + it('forwards ref to the button when rendered without href', () => { + const ref = React.createRef(); + render({anchorText}); + expect(ref.current).toBeInstanceOf(HTMLButtonElement); + }); }); diff --git a/packages/gamut/src/Anchor/index.tsx b/packages/gamut/src/Anchor/index.tsx index 287ff0e6d37..677074fac9a 100644 --- a/packages/gamut/src/Anchor/index.tsx +++ b/packages/gamut/src/Anchor/index.tsx @@ -1,9 +1,19 @@ import { styledOptions, system, variant } from '@codecademy/gamut-styles'; import { StyleProps, variance } from '@codecademy/variance'; import styled from '@emotion/styled'; -import { ComponentProps, forwardRef, HTMLProps, RefObject } from 'react'; +import { + ComponentProps, + ComponentType, + forwardRef, + HTMLProps, + Ref, +} from 'react'; -import { ButtonBase, ButtonSelectors } from '../ButtonBase/ButtonBase'; +import { + ButtonBase, + ButtonSelectors, + narrowButtonBaseRef, +} from '../ButtonBase/ButtonBase'; import { AppendedIconProps, appendIconToContent } from '../helpers'; export interface AnchorProps @@ -107,11 +117,18 @@ const anchorProps = variance.compose( system.typography ); -export const AnchorBase = styled('a', styledOptions<'a'>())( +const AnchorBaseStyled = styled('a', styledOptions<'a'>())( anchorVariants, anchorProps ); +/** AnchorBase ref accepts anchor or button because it can render as ButtonBase when there is no href. */ +export const AnchorBase = AnchorBaseStyled as ComponentType< + Omit, 'ref'> & { + ref?: Ref; + } +>; + type AnchorBaseProps = | ComponentProps | (Exclude, 'ref'> & @@ -150,7 +167,7 @@ export const Anchor = forwardRef< return ( } + ref={narrowButtonBaseRef(ref)} variant={variant} {...rest} > @@ -161,7 +178,7 @@ export const Anchor = forwardRef< return ( } + ref={narrowButtonBaseRef(ref)} variant={variant} {...rest} > diff --git a/packages/gamut/src/BarChart/utils/hooks.ts b/packages/gamut/src/BarChart/utils/hooks.ts index cae93671846..a255e9d8d5a 100644 --- a/packages/gamut/src/BarChart/utils/hooks.ts +++ b/packages/gamut/src/BarChart/utils/hooks.ts @@ -211,7 +211,7 @@ const useMeasureWidth = ({ onMeasure, isMeasuring, }: { - ref: React.RefObject; + ref: React.RefObject; onMeasure: (width: number) => void; isMeasuring: boolean; }): void => { @@ -233,7 +233,7 @@ const useMeasureWidth = ({ } const element = ref.current; - const width = element?.getBoundingClientRect()?.width; + const { width } = element.getBoundingClientRect(); if (width > 0) { onMeasure(width); @@ -245,7 +245,7 @@ const useMeasureWidth = ({ export const useMeasureCategoryLabelWidth = ({ ref, }: { - ref: React.RefObject; + ref: React.RefObject; }): void => { const { setWidestCategoryLabelWidth, isMeasuring } = useBarChartContext(); useMeasureWidth({ @@ -258,7 +258,7 @@ export const useMeasureCategoryLabelWidth = ({ export const useMeasureTotalValueLabelWidth = ({ ref, }: { - ref: React.RefObject; + ref: React.RefObject; }): void => { const { setWidestTotalValueLabelWidth, isMeasuring } = useBarChartContext(); useMeasureWidth({ diff --git a/packages/gamut/src/Box/GridBox.tsx b/packages/gamut/src/Box/GridBox.tsx index b005a768020..67d562fb57c 100644 --- a/packages/gamut/src/Box/GridBox.tsx +++ b/packages/gamut/src/Box/GridBox.tsx @@ -12,3 +12,5 @@ export const GridBox = styled( gridStates, boxProps ); + +export type { GridBoxProps } from './props'; diff --git a/packages/gamut/src/Breadcrumbs/index.tsx b/packages/gamut/src/Breadcrumbs/index.tsx index e7134094ef3..9f8c66ce9c0 100644 --- a/packages/gamut/src/Breadcrumbs/index.tsx +++ b/packages/gamut/src/Breadcrumbs/index.tsx @@ -36,24 +36,24 @@ const BreadcrumbPart = styled(Box)( export type Crumb = { title: string }; -export type ClickableCrumb = Crumb & { +export type ClickableCrumb = Crumb & { href: string; payload: T; }; -export type Breadcrumb = Crumb | ClickableCrumb; +export type Breadcrumb = Crumb | ClickableCrumb; -export const isClickableCrumb = ( +export const isClickableCrumb = ( crumb: Breadcrumb ): crumb is ClickableCrumb => !!(crumb as ClickableCrumb).href; -export type BreadcrumbsProps = { +export type BreadcrumbsProps = { crumbs: Breadcrumb[]; onClick?: (event: React.MouseEvent, crumb: ClickableCrumb) => void; className?: string; }; -export const Breadcrumbs = ({ +export const Breadcrumbs = ({ crumbs, onClick, className, diff --git a/packages/gamut/src/Button/shared/types.ts b/packages/gamut/src/Button/shared/types.ts index 02c84d4e278..95838ba097c 100644 --- a/packages/gamut/src/Button/shared/types.ts +++ b/packages/gamut/src/Button/shared/types.ts @@ -23,16 +23,18 @@ export type ButtonProps = ButtonBaseProps & ComponentProps; export type InlineIconButtonProps< BaseButtonType extends - | keyof JSX.IntrinsicElements + | keyof React.JSX.IntrinsicElements | React.JSXElementConstructor > = ComponentProps & Partial & { iconPosition?: 'right' | 'left'; }; +/* eslint-disable @typescript-eslint/no-duplicate-type-constituents -- createButtonComponent yields structurally identical typeofs; union documents distinct components */ export type ButtonTypes = | typeof CTAButton | typeof FillButton | typeof IconButton | typeof StrokeButton | typeof TextButton; +/* eslint-enable @typescript-eslint/no-duplicate-type-constituents */ diff --git a/packages/gamut/src/ButtonBase/ButtonBase.tsx b/packages/gamut/src/ButtonBase/ButtonBase.tsx index 3441636cf52..5c365befb3e 100644 --- a/packages/gamut/src/ButtonBase/ButtonBase.tsx +++ b/packages/gamut/src/ButtonBase/ButtonBase.tsx @@ -1,12 +1,9 @@ import { css, styledOptions } from '@codecademy/gamut-styles'; import styled from '@emotion/styled'; -import { ComponentProps, forwardRef, HTMLProps, MutableRefObject } from 'react'; +import { ComponentProps, forwardRef, HTMLProps, Ref } from 'react'; export type ButtonBaseElements = HTMLAnchorElement | HTMLButtonElement; -export type ButtonBaseRef = - | ((instance: ButtonBaseElements | null) => void) - | MutableRefObject - | null; +export type ButtonBaseRef = Ref; export type ButtonBaseElementProps = HTMLProps< HTMLAnchorElement | HTMLButtonElement @@ -62,6 +59,16 @@ type ButtonBaseProps = | (Exclude, 'ref'> & ComponentProps); +/** + * Narrows a ref union (anchor | button) to the element type for the current render branch. + * Use when forwarding refs from components that render either an anchor or a button (e.g. ButtonBase, Anchor). + */ +export function narrowButtonBaseRef( + ref: Ref +): Ref { + return ref as Ref; +} + export const ButtonBase = forwardRef< HTMLButtonElement | HTMLAnchorElement, ButtonBaseProps @@ -76,7 +83,7 @@ export const ButtonBase = forwardRef< {...filteredProps} as="button" disabled={!!disabled} - ref={ref as MutableRefObject} + ref={narrowButtonBaseRef(ref)} role={role} type={type} > @@ -90,7 +97,7 @@ export const ButtonBase = forwardRef< {...rest} as="a" href={rest?.href} - ref={ref as MutableRefObject} + ref={narrowButtonBaseRef(ref)} role={role} > {children} diff --git a/packages/gamut/src/ButtonBase/__tests__/ButtonBase.test.tsx b/packages/gamut/src/ButtonBase/__tests__/ButtonBase.test.tsx index 62eeff1b449..60aea38e8a4 100644 --- a/packages/gamut/src/ButtonBase/__tests__/ButtonBase.test.tsx +++ b/packages/gamut/src/ButtonBase/__tests__/ButtonBase.test.tsx @@ -1,4 +1,6 @@ import { setupRtl } from '@codecademy/gamut-tests'; +import { render } from '@testing-library/react'; +import React from 'react'; import { ButtonBase } from '../ButtonBase'; @@ -53,4 +55,20 @@ describe('ButtonBase', () => { expect(el.getAttribute('disabled')).toBe(''); }); }); + + it('forwards ref to the button element', () => { + const ref = React.createRef(); + render({buttonText}); + expect(ref.current).toBeInstanceOf(HTMLButtonElement); + }); + + it('forwards ref to the anchor when href is provided', () => { + const ref = React.createRef(); + render( + + {buttonText} + + ); + expect(ref.current).toBeInstanceOf(HTMLAnchorElement); + }); }); diff --git a/packages/gamut/src/Card/styles.tsx b/packages/gamut/src/Card/styles.tsx index 1a080095ea7..aa64aab6a30 100644 --- a/packages/gamut/src/Card/styles.tsx +++ b/packages/gamut/src/Card/styles.tsx @@ -49,14 +49,14 @@ export const patternFadeInOut = { opacity: 1, transition: { duration: timingValues.medium / 1000, - ease: 'easeOut', + ease: 'easeOut' as const, }, }, animate: { opacity: 0, transition: { duration: timingValues.medium / 1000, - ease: 'easeIn', + ease: 'easeIn' as const, }, }, }; @@ -67,7 +67,7 @@ export const hoverShadowLeft = (borderRadius?: string) => ({ borderRadius, transition: { duration: timingValues.fast / 1000, - ease: 'easeOut', + ease: 'easeOut' as const, }, }, initialOutline: { @@ -75,7 +75,7 @@ export const hoverShadowLeft = (borderRadius?: string) => ({ borderRadius, transition: { duration: timingValues.fast / 1000, - ease: 'easeOut', + ease: 'easeOut' as const, }, }, animate: { @@ -84,7 +84,7 @@ export const hoverShadowLeft = (borderRadius?: string) => ({ borderRadius, transition: { duration: timingValues.fast / 1000, - ease: 'easeIn', + ease: 'easeIn' as const, }, }, animateOutline: { @@ -93,7 +93,7 @@ export const hoverShadowLeft = (borderRadius?: string) => ({ borderRadius, transition: { duration: timingValues.fast / 1000, - ease: 'easeIn', + ease: 'easeIn' as const, }, }, }); @@ -104,7 +104,7 @@ export const hoverShadowRight = (borderRadius?: string) => ({ borderRadius, transition: { duration: timingValues.fast / 1000, - ease: 'easeOut', + ease: 'easeOut' as const, }, }, animate: { @@ -113,7 +113,7 @@ export const hoverShadowRight = (borderRadius?: string) => ({ borderRadius, transition: { duration: timingValues.fast / 1000, - ease: 'easeIn', + ease: 'easeIn' as const, }, }, }); diff --git a/packages/gamut/src/Coachmark/index.tsx b/packages/gamut/src/Coachmark/index.tsx index a4316f6fde3..999c3c0a119 100644 --- a/packages/gamut/src/Coachmark/index.tsx +++ b/packages/gamut/src/Coachmark/index.tsx @@ -30,7 +30,7 @@ export type CoachmarkProps = PopoverFocusProps & { /** * Function that returns the contents of the coachmark. */ - renderPopover: (onDismiss?: () => void) => JSX.Element; + renderPopover: (onDismiss?: () => void) => React.JSX.Element; /** * Props to be passed into the popover component. */ diff --git a/packages/gamut/src/ConnectedForm/ConnectedForm.tsx b/packages/gamut/src/ConnectedForm/ConnectedForm.tsx index 95575145d48..f9fa259d0a3 100644 --- a/packages/gamut/src/ConnectedForm/ConnectedForm.tsx +++ b/packages/gamut/src/ConnectedForm/ConnectedForm.tsx @@ -182,6 +182,6 @@ export const ConnectedForm = forwardRef( ); } ) as >( - props: ConnectedFormProps, - ref: React.ForwardedRef + props: React.PropsWithoutRef> & + React.RefAttributes ) => React.ReactElement; diff --git a/packages/gamut/src/ConnectedForm/ConnectedFormGroup.tsx b/packages/gamut/src/ConnectedForm/ConnectedFormGroup.tsx index 90d5ede6d2d..310d03dc406 100644 --- a/packages/gamut/src/ConnectedForm/ConnectedFormGroup.tsx +++ b/packages/gamut/src/ConnectedForm/ConnectedFormGroup.tsx @@ -82,10 +82,12 @@ export function ConnectedFormGroup({ } }, [customError, name, setError]); + const fieldId = typeof id === 'string' ? id : name; + const renderedLabel = ( ({ const textError = customError || getErrorMessage(error); const showError = !!(textError && !hideLabel); - const errorId = showError ? `${id || name}_error` : undefined; + const errorId = showError ? `${fieldId}_error` : undefined; return ( diff --git a/packages/gamut/src/ConnectedForm/utils.tsx b/packages/gamut/src/ConnectedForm/utils.tsx index 7bf0e6eb651..7330477fa95 100644 --- a/packages/gamut/src/ConnectedForm/utils.tsx +++ b/packages/gamut/src/ConnectedForm/utils.tsx @@ -183,9 +183,7 @@ export const useField = ({ }); const formValidation = - (validationRules && - validationRules[name as keyof typeof validationRules]) ?? - undefined; + validationRules?.[name as keyof typeof validationRules]; const validation = useMemo( () => @@ -347,7 +345,6 @@ type InputTypes = | 'search' | 'month' | 'tel' - | 'time' | 'url' | 'week' > diff --git a/packages/gamut/src/DatePicker/DatePickerInput/index.tsx b/packages/gamut/src/DatePicker/DatePickerInput/index.tsx index ebce7088a2e..cde2efbab80 100644 --- a/packages/gamut/src/DatePicker/DatePickerInput/index.tsx +++ b/packages/gamut/src/DatePicker/DatePickerInput/index.tsx @@ -20,10 +20,10 @@ import { SegmentedShell } from './elements'; import { DatePickerInputSegment } from './Segment'; import { SegmentLiteral } from './Segment/elements'; import { - type SegmentValues, getDateSegmentsFromDate, normalizeSegmentValues, parseSegmentsToDate, + type SegmentValues, } from './Segment/utils'; import { type DatePartKind, diff --git a/packages/gamut/src/FeatureShimmer/__tests__/FeatureShimmer.test.tsx b/packages/gamut/src/FeatureShimmer/__tests__/FeatureShimmer.test.tsx index c5bb6d7704a..b88f143c656 100644 --- a/packages/gamut/src/FeatureShimmer/__tests__/FeatureShimmer.test.tsx +++ b/packages/gamut/src/FeatureShimmer/__tests__/FeatureShimmer.test.tsx @@ -17,7 +17,9 @@ describe('FeatureShimmer', () => { beforeEach(() => { mockIntersectionObserver.mockReturnValue({ observe: jest.fn(), + unobserve: jest.fn(), disconnect: jest.fn(), + unobserve: jest.fn(), }); window.IntersectionObserver = mockIntersectionObserver; }); diff --git a/packages/gamut/src/FeatureShimmer/index.tsx b/packages/gamut/src/FeatureShimmer/index.tsx index 78218b1ceca..21b2ad849c7 100644 --- a/packages/gamut/src/FeatureShimmer/index.tsx +++ b/packages/gamut/src/FeatureShimmer/index.tsx @@ -24,7 +24,7 @@ const boxVariants = { backgroundColor: 'rgba(0, 0, 0, 0)', borderColor: 'rgba(0, 0, 0, 0)', transition: { - ease: 'easeOut', + ease: 'easeOut' as const, duration: 0.3, delay: 4, }, @@ -37,12 +37,12 @@ const shimmerVariants = { backgroundColor: 'rgba(0, 0, 0, 0)', transition: { left: { - ease: 'easeInOut', + ease: 'easeInOut' as const, duration: 2, delay: 2, }, backgroundColor: { - ease: 'easeOut', + ease: 'easeOut' as const, duration: 1, delay: 4, }, diff --git a/packages/gamut/src/Form/SelectDropdown/SelectDropdown.tsx b/packages/gamut/src/Form/SelectDropdown/SelectDropdown.tsx index f31839eaa46..d2a3e8d36d7 100644 --- a/packages/gamut/src/Form/SelectDropdown/SelectDropdown.tsx +++ b/packages/gamut/src/Form/SelectDropdown/SelectDropdown.tsx @@ -1,63 +1,21 @@ import { useTheme } from '@emotion/react'; -import { - KeyboardEvent, - useCallback, - useEffect, - useId, - useMemo, - useRef, - useState, -} from 'react'; +import { useId, useMemo, useRef, useState } from 'react'; import * as React from 'react'; -import { Options as OptionsType, StylesConfig } from 'react-select'; +import { StylesConfig } from 'react-select'; -import { parseOptions, SelectOptionBase } from '../utils'; +import { onFocus } from './core/accessibility'; +import { defaultComponents } from './core/constants'; +import { getMemoizedStyles } from './core/styles'; +import { resolveNoOptionsMessage } from './core/utils'; import { - AbbreviatedSingleValue, - CustomContainer, - CustomInput, - CustomValueContainer, - DropdownButton, formatGroupLabel, formatOptionLabel, - IconOption, - MultiValueRemoveButton, - MultiValueWithColorMode, - onFocus, - RemoveAllButton, SelectDropdownContext, TypedReactSelect, } from './elements'; -import { getMemoizedStyles } from './styles'; -import { - OptionStrict, - SelectDropdownGroup, - SelectDropdownProps, -} from './types'; -import { - filterValueFromOptions, - isMultipleSelectProps, - isOptionsGrouped, - isSingleSelectProps, - removeValueFromSelectedOptions, -} from './utils'; - -const defaultProps = { - name: undefined, - components: { - DropdownIndicator: DropdownButton, - IndicatorSeparator: () => null, - ClearIndicator: RemoveAllButton, - SelectContainer: CustomContainer, - ValueContainer: CustomValueContainer, - MultiValue: MultiValueWithColorMode, - MultiValueRemove: MultiValueRemoveButton, - Option: IconOption, - SingleValue: AbbreviatedSingleValue, - Input: CustomInput, - }, -}; -const onChangeAction = 'select-option'; +import { useSelectHandlers } from './hooks/useSelectHandlers'; +import { useSelectOptions } from './hooks/useSelectOptions'; +import { SelectDropdownProps } from './types'; /** * A flexible dropdown select component built on top of react-select. @@ -109,138 +67,55 @@ export const SelectDropdown: React.FC = ({ disabled, dropdownWidth, error, + formatCreateLabel = (inputValue: string) => `Add "${inputValue}"`, id, inputProps, inputWidth, - isSearchable = false, + isCreatable = false, + isSearchable: isSearchableProp = false, + isValidNewOption, menuAlignment = 'left', multiple, name, onChange, + onCreateOption, + onInputChange, options, placeholder = 'Select an option', shownOptionsLimit = 6, size, + validationMessage, value, zIndex, ...rest }) => { + // isSearchable is forced true when isCreatable is true (CreatableSelect requires a text input) + const isSearchable = isCreatable || isSearchableProp; const rawInputId = useId(); const inputId = name ?? `${id}-select-dropdown-${rawInputId}`; - const [activated, setActivated] = useState(false); - const [currentFocusedValue, setCurrentFocusedValue] = useState(undefined); - - // these are used to programatically manage the focus state of our multi-select options + 'Remove all' button const removeAllButtonRef = useRef(null); const selectInputRef = useRef(null); - const selectOptions = useMemo((): - | SelectOptionBase[] - | SelectDropdownGroup[] => { - if ( - !options || - (Array.isArray(options) && !options.length) || - (typeof options === 'object' && - !Array.isArray(options) && - Object.keys(options).length === 0) - ) { - return []; - } - - if (isOptionsGrouped(options)) { - return options; - } - - return parseOptions({ options, id, size }); - }, [options, id, size]); - - const parsedValue = useMemo(() => { - if (isOptionsGrouped(selectOptions)) { - for (const group of selectOptions) { - if (group.options) { - const foundOption = group.options.find( - (option) => option.value === value - ); - if (foundOption) return foundOption; - } - } - return undefined; - } - - return selectOptions.find((option) => option.value === value); - }, [selectOptions, value]); - - const [multiValues, setMultiValues] = useState( - multiple && // To keep this efficient for non-multiSelect - filterValueFromOptions( - selectOptions, - value, - isOptionsGrouped(selectOptions) - ) - ); + const [currentFocusedValue, setCurrentFocusedValue] = useState(undefined); - // If the caller changes the initial value, let's update our value to match. - useEffect(() => { - const newMultiValues = filterValueFromOptions( + const { selectOptions, parsedValue } = useSelectOptions({ + options, + id, + size, + value: value as string | string[] | undefined, + }); + + const { activated, multiValues, changeHandler, keyPressHandler } = + useSelectHandlers({ + onChange, + multiple, + onCreateOption, selectOptions, value, - isOptionsGrouped(selectOptions) - ); - if (newMultiValues !== multiValues) setMultiValues(newMultiValues); - - // - // We only update this when our passed in options or value changes, not multiValues. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [options, value]); - - const changeHandler = useCallback( - (optionEvent: OptionStrict | OptionsType) => { - setActivated(true); - - // We have to do this because the version of typescript we have doesn't have the transitivity of these type guards yet. But, we will soon! - // Should probably come with: https://codecademy.atlassian.net/browse/GM-354 - const onChangeProps = { onChange, multiple }; - - if (isSingleSelectProps(onChangeProps)) { - const singleOptionEvent = optionEvent as OptionStrict; - - onChangeProps.onChange?.(singleOptionEvent, { - action: onChangeAction, - option: singleOptionEvent, - }); - } - - if (isMultipleSelectProps(onChangeProps)) { - setMultiValues(optionEvent as OptionStrict[]); - - onChangeProps.onChange?.(optionEvent as OptionsType, { - action: onChangeAction, - option: undefined, // At the moment this isn't used, but when multi select is built for real, boom (https://codecademy.atlassian.net/browse/GM-354) - }); - } - }, - [onChange, multiple] - ); - - const keyPressHandler = (e: KeyboardEvent) => { - if (multiple && e.key === 'Enter' && currentFocusedValue && multiValues) { - const newMultiValues = removeValueFromSelectedOptions( - multiValues, - currentFocusedValue - ); - - if (newMultiValues !== multiValues) setMultiValues(newMultiValues); - } - if ( - removeAllButtonRef.current !== null && - e.key === 'ArrowRight' && - multiValues && - currentFocusedValue === multiValues[multiValues.length - 1].value - ) { - removeAllButtonRef.current.focus(); - } - }; + currentFocusedValue, + removeAllButtonRef, + }); const theme = useTheme(); const memoizedStyles = useMemo((): StylesConfig => { @@ -257,26 +132,28 @@ export const SelectDropdown: React.FC = ({ }} > option.disabled} isSearchable={isSearchable} + isValidNewOption={isValidNewOption} menuAlignment={menuAlignment} name={name} + noOptionsMessage={resolveNoOptionsMessage(validationMessage)} options={selectOptions} placeholder={placeholder} selectRef={selectInputRef} @@ -285,7 +162,8 @@ export const SelectDropdown: React.FC = ({ styles={memoizedStyles} value={multiple ? multiValues : parsedValue} onChange={changeHandler} - onKeyDown={multiple ? (e) => keyPressHandler(e) : undefined} + onInputChange={onInputChange} + onKeyDown={multiple ? keyPressHandler : undefined} {...rest} /> diff --git a/packages/gamut/src/Form/SelectDropdown/__tests__/utils.test.tsx b/packages/gamut/src/Form/SelectDropdown/__tests__/utils.test.tsx index 6e3ce321144..dcc9f3270d3 100644 --- a/packages/gamut/src/Form/SelectDropdown/__tests__/utils.test.tsx +++ b/packages/gamut/src/Form/SelectDropdown/__tests__/utils.test.tsx @@ -1,13 +1,72 @@ import { SelectOptionBase } from '../../utils'; -import { ExtendedOption, SelectDropdownGroup } from '../types'; import { filterValueFromOptions, + getCreatedOptionValue, isOptionGroup, isOptionsGrouped, removeValueFromSelectedOptions, -} from '../utils'; +} from '../core/utils'; +import { ExtendedOption, SelectDropdownGroup } from '../types'; describe('SelectDropdown Utils', () => { + describe('getCreatedOptionValue', () => { + it('returns actionMeta.option.value when present', () => { + expect( + getCreatedOptionValue( + { label: 'Purple', value: 'purple' }, + { + action: 'create-option', + option: { label: 'Purple', value: 'purple' }, + }, + false + ) + ).toBe('purple'); + }); + + it('falls back to single-select optionEvent value when actionMeta.option is missing', () => { + expect( + getCreatedOptionValue( + { label: 'Purple', value: 'purple' }, + { action: 'create-option' }, + false + ) + ).toBe('purple'); + }); + + it('falls back to multi-select __isNew__ option when actionMeta.option is missing', () => { + expect( + getCreatedOptionValue( + [ + { label: 'Red', value: 'red' }, + { label: 'Purple', value: 'purple', __isNew__: true }, + ], + { action: 'create-option' }, + true + ) + ).toBe('purple'); + }); + + it('returns undefined when no created value can be resolved', () => { + expect( + getCreatedOptionValue( + [{ label: 'Red', value: 'red' }], + { action: 'create-option' }, + true + ) + ).toBeUndefined(); + }); + + it('returns undefined for an empty actionMeta.option.value', () => { + expect( + getCreatedOptionValue( + { label: '', value: '' }, + { action: 'create-option', option: { label: '', value: '' } }, + false + ) + ).toBeUndefined(); + }); + }); + describe('isOptionGroup', () => { it('returns true for valid group objects', () => { const groupOption: SelectDropdownGroup = { diff --git a/packages/gamut/src/Form/SelectDropdown/core/accessibility.ts b/packages/gamut/src/Form/SelectDropdown/core/accessibility.ts new file mode 100644 index 00000000000..0ab3a71fb69 --- /dev/null +++ b/packages/gamut/src/Form/SelectDropdown/core/accessibility.ts @@ -0,0 +1,14 @@ +import { AriaOnFocus } from 'react-select'; + +import { ExtendedOption } from '../types'; + +export const onFocus: AriaOnFocus = ({ + focused: { label, subtitle, rightLabel, disabled }, +}) => { + const formattedSubtitle = `, ${subtitle}`; + const formattedRightLabel = `, ${rightLabel}`; + + return `You are currently focused on option ${label}${ + subtitle ? formattedSubtitle : '' + } ${rightLabel ? formattedRightLabel : ''}${disabled ? ', disabled' : ''}`; +}; diff --git a/packages/gamut/src/Form/SelectDropdown/core/constants.ts b/packages/gamut/src/Form/SelectDropdown/core/constants.ts new file mode 100644 index 00000000000..c21286fab5b --- /dev/null +++ b/packages/gamut/src/Form/SelectDropdown/core/constants.ts @@ -0,0 +1,26 @@ +import { + AbbreviatedSingleValue, + CustomContainer, + CustomInput, + CustomValueContainer, + DropdownButton, + IconOption, + MultiValueRemoveButton, + MultiValueWithColorMode, + RemoveAllButton, +} from '../elements'; + +export const defaultComponents = { + DropdownIndicator: DropdownButton, + IndicatorSeparator: () => null, + ClearIndicator: RemoveAllButton, + SelectContainer: CustomContainer, + ValueContainer: CustomValueContainer, + MultiValue: MultiValueWithColorMode, + MultiValueRemove: MultiValueRemoveButton, + Option: IconOption, + SingleValue: AbbreviatedSingleValue, + Input: CustomInput, +}; + +export const ON_CHANGE_ACTION = 'select-option' as const; diff --git a/packages/gamut/src/Form/SelectDropdown/styles.ts b/packages/gamut/src/Form/SelectDropdown/core/styles.ts similarity index 81% rename from packages/gamut/src/Form/SelectDropdown/styles.ts rename to packages/gamut/src/Form/SelectDropdown/core/styles.ts index ba9178c113c..605a73b7e8c 100644 --- a/packages/gamut/src/Form/SelectDropdown/styles.ts +++ b/packages/gamut/src/Form/SelectDropdown/core/styles.ts @@ -11,16 +11,16 @@ import { tagBaseStyles, tagLabelFontSize, tagLabelPadding, -} from '../../Tag/styles'; +} from '../../../Tag/styles'; import { formBaseComponentStyles, formBaseFieldStylesObject, formFieldDisabledStyles, formFieldPaddingStyles, InputSelectors, -} from '../styles'; -import { ControlState, OptionState } from './types'; -import { BaseSelectComponentProps } from './types/styles'; +} from '../../styles'; +import { ControlState, OptionState } from '../types'; +import { BaseSelectComponentProps } from '../types/styles'; const selectDropdownStyles = css({ ...formBaseFieldStylesObject, @@ -86,7 +86,7 @@ const textColor = css({ }); const placeholderColor = css({ - color: 'text-disabled', + color: 'text-secondary', }); export const getMemoizedStyles = ( @@ -150,6 +150,8 @@ export const getMemoizedStyles = ( ...provided, ...dropdownBorderStyles(zIndex)({ theme }), ...dropdownBorderStates({ error: state.selectProps.error, theme }), + // Drop react-select's default menu drop shadow; the border above defines the edge. + boxShadow: 'none', ...(dropdownWidth ? { minWidth: dropdownWidth, @@ -211,14 +213,34 @@ export const getMemoizedStyles = ( backgroundColor: theme.colors['secondary-hover'], }, }), - option: (provided, state: OptionState) => ({ - ...getOptionBackground(state.isSelected, state.isFocused)({ theme }), - alignItems: 'center', - color: state.isDisabled ? 'text-disabled' : 'default', - cursor: state.isDisabled ? 'not-allowed' : 'pointer', - display: 'flex', - padding: state.selectProps.size === 'small' ? '3px 14px' : '11px 14px', + noOptionsMessage: (provided) => ({ + ...provided, + color: theme.colors['text-secondary'], }), + option: (provided, state: OptionState) => { + const isNew = state.data?.__isNew__; + const isSmall = state.selectProps.size === 'small'; + return { + ...getOptionBackground(state.isSelected, state.isFocused)({ theme }), + alignItems: 'center', + color: state.isDisabled + ? theme.colors['text-disabled'] + : isNew + ? theme.colors.primary + : theme.colors.text, + cursor: state.isDisabled ? 'not-allowed' : 'pointer', + display: 'flex', + padding: isSmall ? '3px 14px' : '11px 14px', + ...(isNew && { + // Gradient creates the 1px divider line at the top edge of the option background + backgroundImage: `linear-gradient(${theme.colors['text-disabled']} 1px, transparent 1px)`, + backgroundPosition: '0 0', + backgroundRepeat: 'no-repeat', + backgroundSize: '100% 1px', + paddingTop: isSmall ? '11px' : '19px', + }), + }; + }, placeholder: (provided) => ({ ...provided, ...placeholderColor({ theme }), diff --git a/packages/gamut/src/Form/SelectDropdown/utils.tsx b/packages/gamut/src/Form/SelectDropdown/core/utils.ts similarity index 63% rename from packages/gamut/src/Form/SelectDropdown/utils.tsx rename to packages/gamut/src/Form/SelectDropdown/core/utils.ts index bcf0fd7ac87..a32f70b491f 100644 --- a/packages/gamut/src/Form/SelectDropdown/utils.tsx +++ b/packages/gamut/src/Form/SelectDropdown/core/utils.ts @@ -1,13 +1,17 @@ -import { SelectOptionBase } from '../utils'; +import * as React from 'react'; +import { ActionMeta, Options as OptionsType } from 'react-select'; + +import { SelectOptionBase } from '../../utils'; import { BaseOnChangeProps, ExtendedOption, MultiSelectDropdownProps, + OptionStrict, SelectDropdownGroup, SelectDropdownOptions, SelectDropdownProps, SingleSelectDropdownProps, -} from './types'; +} from '../types'; export const isMultipleSelectProps = ( props: BaseOnChangeProps @@ -17,6 +21,32 @@ export const isSingleSelectProps = ( props: BaseOnChangeProps ): props is SingleSelectDropdownProps => !props.multiple; +type CreatableOption = OptionStrict & { __isNew__?: boolean }; + +/** + * Resolves the value for a newly created option from react-select action metadata + * or the onChange option payload. Returns undefined when no reliable value exists. + */ +export const getCreatedOptionValue = ( + optionEvent: OptionStrict | OptionsType, + actionMeta: ActionMeta, + multiple?: boolean +): string | undefined => { + const metaValue = actionMeta.option?.value; + if (metaValue) return metaValue; + + if (!multiple) { + const { value } = optionEvent as OptionStrict; + return value || undefined; + } + + const newOption = (optionEvent as OptionsType).find( + (option) => (option as CreatableOption).__isNew__ + ); + + return newOption?.value || undefined; +}; + export const isOptionGroup = (obj: unknown): obj is SelectDropdownGroup => obj != null && typeof obj === 'object' && @@ -68,6 +98,18 @@ export const filterValueFromOptions = ( * @param value - The value or values to remove * @returns New array with the specified values removed */ +export const resolveNoOptionsMessage = ( + validationMessage: SelectDropdownProps['validationMessage'] +): ((obj: { inputValue: string }) => React.ReactNode) | undefined => { + if (validationMessage === undefined) return undefined; + if (typeof validationMessage === 'function') { + return validationMessage as (obj: { + inputValue: string; + }) => React.ReactNode; + } + return () => validationMessage; +}; + export const removeValueFromSelectedOptions = ( selectedOptions: ExtendedOption[] | SelectOptionBase[], value: SelectDropdownProps['value'] diff --git a/packages/gamut/src/Form/SelectDropdown/elements/constants.ts b/packages/gamut/src/Form/SelectDropdown/elements/constants.ts index 7d34a7f0fb0..f12c8d6c3f2 100644 --- a/packages/gamut/src/Form/SelectDropdown/elements/constants.ts +++ b/packages/gamut/src/Form/SelectDropdown/elements/constants.ts @@ -3,7 +3,6 @@ import { CloseIcon, MiniChevronDownIcon, MiniDeleteIcon, - SearchIcon, } from '@codecademy/gamut-icons'; export const iconSize = { small: 12, medium: 16 }; @@ -18,14 +17,6 @@ export const indicatorIcons = { size: iconSize.medium, icon: ArrowChevronDownIcon, }, - smallSearchable: { - size: iconSize.small, - icon: SearchIcon, - }, - mediumSearchable: { - size: iconSize.medium, - icon: SearchIcon, - }, smallRemove: { size: iconSize.small, icon: MiniDeleteIcon, diff --git a/packages/gamut/src/Form/SelectDropdown/elements/containers.tsx b/packages/gamut/src/Form/SelectDropdown/elements/containers.tsx index 47dd615a032..eb4e15c1abf 100644 --- a/packages/gamut/src/Form/SelectDropdown/elements/containers.tsx +++ b/packages/gamut/src/Form/SelectDropdown/elements/containers.tsx @@ -4,6 +4,7 @@ import ReactSelect, { GroupBase, Props, } from 'react-select'; +import CreatableSelect from 'react-select/creatable'; import { CustomSelectComponentProps, @@ -74,7 +75,7 @@ export const CustomValueContainer = ({ const { inputId } = rest.selectProps; if (inputId) { const inputElement = document.getElementById(inputId); - if (inputElement && inputElement.getAttribute('role') === 'combobox') { + if (inputElement?.getAttribute('role') === 'combobox') { Object.entries(comboboxProps).forEach(([key, value]) => { inputElement.setAttribute(key, value); }); @@ -120,7 +121,11 @@ export const CustomInput = ({ /** * Typed wrapper around react-select component. - * Provides type safety for the underlying react-select implementation. + * Renders CreatableSelect when isCreatable is true, ReactSelect otherwise. + * Creatable-only props (formatCreateLabel, isValidNewOption) are stripped from + * the non-creatable path so they don't reach ReactSelect. `onCreateOption` is + * handled in SelectDropdown's changeHandler — do not pass it to CreatableSelect + * or react-select will skip onChange on create. */ export function TypedReactSelect< OptionType, @@ -128,7 +133,20 @@ export function TypedReactSelect< GroupType extends GroupBase = GroupBase >({ selectRef, + isCreatable, + formatCreateLabel, + isValidNewOption, ...props }: Props & TypedReactSelectProps) { + if (isCreatable) { + return ( + + ); + } return ; } diff --git a/packages/gamut/src/Form/SelectDropdown/elements/controls.tsx b/packages/gamut/src/Form/SelectDropdown/elements/controls.tsx index 3152ed97e2c..e2985b60b3f 100644 --- a/packages/gamut/src/Form/SelectDropdown/elements/controls.tsx +++ b/packages/gamut/src/Form/SelectDropdown/elements/controls.tsx @@ -1,47 +1,19 @@ -import { css, theme } from '@codecademy/gamut-styles'; -import styled from '@emotion/styled'; -import { KeyboardEvent, useContext } from 'react'; -import { - AriaOnFocus, - components as SelectDropdownElements, -} from 'react-select'; +import { components as SelectDropdownElements } from 'react-select'; -import { ExtendedOption, SizedIndicatorProps } from '../types'; +import { SizedIndicatorProps } from '../types'; import { indicatorIcons } from './constants'; -import { SelectDropdownContext } from './containers'; const { DropdownIndicator } = SelectDropdownElements; -/** - * Generates accessible focus messages for screen readers. - * Provides detailed information about the currently focused option. - * - * @param params - Object containing the focused option details - * @returns Formatted accessibility message - */ -export const onFocus: AriaOnFocus = ({ - focused: { label, subtitle, rightLabel, disabled }, -}) => { - const formattedSubtitle = `, ${subtitle}`; - const formattedRightLabel = `, ${rightLabel}`; - - const msg = `You are currently focused on option ${label}${ - subtitle ? formattedSubtitle : '' - } ${rightLabel ? formattedRightLabel : ''}${disabled ? ', disabled' : ''}`; - - return msg; -}; - /** * Custom dropdown indicator that shows either a chevron or search icon. * The icon type depends on whether the select is searchable or not. */ export const DropdownButton = (props: SizedIndicatorProps) => { - const { size, isSearchable } = props.selectProps; + const { size } = props.selectProps; const color = props.isDisabled ? 'text-disabled' : 'text'; const iconSize = size ?? 'medium'; - const iconType = isSearchable ? 'Searchable' : 'Chevron'; - const { ...iconProps } = indicatorIcons[`${iconSize}${iconType}`]; + const { ...iconProps } = indicatorIcons[`${iconSize}Chevron`]; const { icon: IndicatorIcon } = iconProps; return ( @@ -50,64 +22,3 @@ export const DropdownButton = (props: SizedIndicatorProps) => { ); }; - -const CustomStyledRemoveAllDiv = styled('div')( - css({ - '&:focus': { - outline: `2px solid ${theme.colors.primary}`, - }, - '&:focus-visible': { - outline: `2px solid ${theme.colors.primary}`, - }, - }) -); - -/** - * Custom remove all button for multi-select mode. - * Provides keyboard navigation and accessible removal of all selected values. - */ -export const RemoveAllButton = (props: SizedIndicatorProps) => { - const { - getStyles, - innerProps: { ...restInnerProps }, - selectProps: { size }, - } = props; - - const { removeAllButtonRef, selectInputRef } = useContext( - SelectDropdownContext - ); - - const iconSize = size ?? 'medium'; - const { ...iconProps } = indicatorIcons[`${iconSize}Remove`]; - const { icon: IndicatorIcon } = iconProps; - - const onKeyPress = (e: KeyboardEvent) => { - if (e.key === 'Enter' && restInnerProps.onMouseDown) { - restInnerProps.onMouseDown(e as any); - } - - if ( - selectInputRef?.current && - (e.key === 'ArrowRight' || e.key === 'ArrowLeft' || e.key === 'ArrowDown') - ) { - selectInputRef?.current.focus(); - } - }; - - const style = getStyles('clearIndicator', props) as React.CSSProperties; - - return ( - - - - ); -}; diff --git a/packages/gamut/src/Form/SelectDropdown/elements/index.ts b/packages/gamut/src/Form/SelectDropdown/elements/index.ts index a4d695a56fd..7b9dd9e40c7 100644 --- a/packages/gamut/src/Form/SelectDropdown/elements/index.ts +++ b/packages/gamut/src/Form/SelectDropdown/elements/index.ts @@ -6,7 +6,8 @@ export { RemoveAllButton, } from './multi-value'; -export { DropdownButton, onFocus } from './controls'; +export { DropdownButton } from './controls'; +export { onFocus } from '../core/accessibility'; export { SelectDropdownContext, diff --git a/packages/gamut/src/Form/SelectDropdown/elements/multi-value.tsx b/packages/gamut/src/Form/SelectDropdown/elements/multi-value.tsx index bb1d2002565..3744a66e378 100644 --- a/packages/gamut/src/Form/SelectDropdown/elements/multi-value.tsx +++ b/packages/gamut/src/Form/SelectDropdown/elements/multi-value.tsx @@ -109,7 +109,7 @@ export const RemoveAllButton = (props: SizedIndicatorProps) => { selectInputRef?.current && (e.key === 'ArrowRight' || e.key === 'ArrowLeft' || e.key === 'ArrowDown') ) { - selectInputRef?.current.focus(); + selectInputRef.current.focus(); } }; @@ -121,7 +121,7 @@ export const RemoveAllButton = (props: SizedIndicatorProps) => { role="button" tabIndex={0} {...restInnerProps} - ref={removeAllButtonRef} + ref={removeAllButtonRef as React.Ref} // eslint-disable-next-line gamut/no-inline-style style={style} onKeyDown={onKeyPress} diff --git a/packages/gamut/src/Form/SelectDropdown/elements/options.tsx b/packages/gamut/src/Form/SelectDropdown/elements/options.tsx index b3df15e2c0a..2a3bf1ce154 100644 --- a/packages/gamut/src/Form/SelectDropdown/elements/options.tsx +++ b/packages/gamut/src/Form/SelectDropdown/elements/options.tsx @@ -46,13 +46,15 @@ const IconOptionLabel: React.FC< /** * Custom option component that displays a check icon for selected items. * Also manages ARIA attributes for accessibility. + * Skips the check icon for react-select/creatable's "Add" row (__isNew__). */ export const IconOption = ({ children, ...rest }: CustomSelectComponentProps) => { const { size } = rest.selectProps; - const { isFocused, innerProps } = rest; + const { isFocused, innerProps, data } = rest; + const isNew = (data as any)?.__isNew__; return ( {children} - {rest?.isSelected && ( + {!isNew && rest?.isSelected && ( )} diff --git a/packages/gamut/src/Form/SelectDropdown/hooks/useSelectHandlers.ts b/packages/gamut/src/Form/SelectDropdown/hooks/useSelectHandlers.ts new file mode 100644 index 00000000000..ff779df7cea --- /dev/null +++ b/packages/gamut/src/Form/SelectDropdown/hooks/useSelectHandlers.ts @@ -0,0 +1,142 @@ +import { KeyboardEvent, useCallback, useEffect, useState } from 'react'; +import * as React from 'react'; +import { ActionMeta, Options as OptionsType } from 'react-select'; + +import { SelectOptionBase } from '../../utils'; +import { ON_CHANGE_ACTION } from '../core/constants'; +import { + filterValueFromOptions, + getCreatedOptionValue, + isMultipleSelectProps, + isOptionsGrouped, + isSingleSelectProps, + removeValueFromSelectedOptions, +} from '../core/utils'; +import { + MultiSelectDropdownProps, + OptionStrict, + SelectDropdownGroup, + SelectDropdownProps, + SingleSelectDropdownProps, +} from '../types'; + +interface UseSelectHandlersArgs { + onChange?: + | SingleSelectDropdownProps['onChange'] + | MultiSelectDropdownProps['onChange']; + multiple?: boolean; + onCreateOption?: (inputValue: string) => void; + selectOptions: SelectOptionBase[] | SelectDropdownGroup[]; + value?: SelectDropdownProps['value']; + currentFocusedValue: unknown; + removeAllButtonRef: React.MutableRefObject; +} + +interface UseSelectHandlersReturn { + activated: boolean; + multiValues: OptionStrict[] | false; + changeHandler: ( + optionEvent: OptionStrict | OptionsType, + actionMeta: ActionMeta + ) => void; + keyPressHandler: (e: KeyboardEvent) => void; +} + +export const useSelectHandlers = ({ + onChange, + multiple, + onCreateOption, + selectOptions, + value, + currentFocusedValue, + removeAllButtonRef, +}: UseSelectHandlersArgs): UseSelectHandlersReturn => { + const [activated, setActivated] = useState(false); + const [multiValues, setMultiValues] = useState( + multiple + ? filterValueFromOptions( + selectOptions, + value, + isOptionsGrouped(selectOptions) + ) + : false + ); + + // Sync multi-select value from props when controlled (`value` is a string[]). + // Uncontrolled multi (`value` undefined or '') keeps selection in local state. + useEffect(() => { + if (!multiple || !Array.isArray(value)) return; + + const newMultiValues = filterValueFromOptions( + selectOptions, + value, + isOptionsGrouped(selectOptions) + ); + if (newMultiValues !== multiValues) setMultiValues(newMultiValues); + + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [selectOptions, value, multiple]); + + const changeHandler = useCallback( + ( + optionEvent: OptionStrict | OptionsType, + actionMeta: ActionMeta + ) => { + setActivated(true); + + if (actionMeta.action === 'create-option') { + const createdValue = getCreatedOptionValue( + optionEvent, + actionMeta, + multiple + ); + if (createdValue) onCreateOption?.(createdValue); + } + + const onChangeProps = { onChange, multiple }; + const forwardedMeta: ActionMeta = + actionMeta.action === 'create-option' + ? actionMeta + : { + action: ON_CHANGE_ACTION, + option: isMultipleSelectProps(onChangeProps) + ? undefined + : (optionEvent as OptionStrict), + }; + + if (isSingleSelectProps(onChangeProps)) { + onChangeProps.onChange?.(optionEvent as OptionStrict, forwardedMeta); + } + + if (isMultipleSelectProps(onChangeProps)) { + setMultiValues(optionEvent as OptionStrict[]); + onChangeProps.onChange?.( + optionEvent as OptionsType, + forwardedMeta + ); + } + }, + [onChange, multiple, onCreateOption] + ); + + const keyPressHandler = (e: KeyboardEvent) => { + if (multiple && e.key === 'Enter' && currentFocusedValue && multiValues) { + const newMultiValues = removeValueFromSelectedOptions( + multiValues, + currentFocusedValue as SelectDropdownProps['value'] + ); + if (newMultiValues !== multiValues) setMultiValues(newMultiValues); + } + + if ( + removeAllButtonRef.current !== null && + e.key === 'ArrowRight' && + multiValues && + currentFocusedValue === multiValues[multiValues.length - 1].value + ) { + removeAllButtonRef.current.focus(); + } + }; + + return { activated, multiValues, changeHandler, keyPressHandler }; +}; diff --git a/packages/gamut/src/Form/SelectDropdown/hooks/useSelectOptions.ts b/packages/gamut/src/Form/SelectDropdown/hooks/useSelectOptions.ts new file mode 100644 index 00000000000..200bcb40744 --- /dev/null +++ b/packages/gamut/src/Form/SelectDropdown/hooks/useSelectOptions.ts @@ -0,0 +1,66 @@ +import { useMemo } from 'react'; + +import { parseOptions, SelectOptionBase } from '../../utils'; +import { isOptionsGrouped } from '../core/utils'; +import { + SelectDropdownGroup, + SelectDropdownOptions, + SelectDropdownSizes, +} from '../types'; + +interface UseSelectOptionsArgs { + options?: SelectDropdownOptions | SelectDropdownGroup[]; + id?: string; + size?: SelectDropdownSizes['size']; + value?: string | string[]; +} + +interface UseSelectOptionsReturn { + selectOptions: SelectOptionBase[] | SelectDropdownGroup[]; + parsedValue: SelectOptionBase | undefined; +} + +export const useSelectOptions = ({ + options, + id, + size, + value, +}: UseSelectOptionsArgs): UseSelectOptionsReturn => { + const selectOptions = useMemo((): + | SelectOptionBase[] + | SelectDropdownGroup[] => { + if ( + !options || + (Array.isArray(options) && !options.length) || + (typeof options === 'object' && + !Array.isArray(options) && + Object.keys(options).length === 0) + ) { + return []; + } + + if (isOptionsGrouped(options)) { + return options; + } + + return parseOptions({ options, id, size }); + }, [options, id, size]); + + const parsedValue = useMemo(() => { + if (isOptionsGrouped(selectOptions)) { + for (const group of selectOptions) { + if (group.options) { + const foundOption = group.options.find( + (option) => option.value === value + ); + if (foundOption) return foundOption as SelectOptionBase; + } + } + return undefined; + } + + return selectOptions.find((option) => option.value === value); + }, [selectOptions, value]); + + return { selectOptions, parsedValue }; +}; diff --git a/packages/gamut/src/Form/SelectDropdown/types/component-props.ts b/packages/gamut/src/Form/SelectDropdown/types/component-props.ts index d5c89453083..91c18d7e057 100644 --- a/packages/gamut/src/Form/SelectDropdown/types/component-props.ts +++ b/packages/gamut/src/Form/SelectDropdown/types/component-props.ts @@ -1,5 +1,5 @@ import { Ref, SelectHTMLAttributes } from 'react'; -import { Props as NamedProps } from 'react-select'; +import { Options as OptionsType, Props as NamedProps } from 'react-select'; import { SelectComponentProps } from '../../inputs/Select'; import { @@ -58,6 +58,7 @@ export interface SelectDropdownCoreProps | 'theme' | 'onChange' | 'multiple' + | 'isSearchable' >, Pick< SelectHTMLAttributes, @@ -73,6 +74,43 @@ export interface SelectDropdownCoreProps placeholder?: string; /** Array of options or option groups to display in the dropdown */ options?: SelectDropdownOptions | SelectDropdownGroup[]; + /** + * Allows users to create new options by typing a value not in the options list. + * When true, isSearchable is automatically set to true. + * Pair with onCreateOption to persist new options. + */ + isCreatable?: boolean; + /** + * Called when the user confirms a new option via the "Add" row. + * Convenience callback for persisting the new value to your `options` list. + * Selection updates are delivered through `onChange` with `action: 'create-option'`. + */ + onCreateOption?: (inputValue: string) => void; + /** + * Customises the label shown in the "Add" row. + * Defaults to: (inputValue) => `Add "${inputValue}"`. + */ + formatCreateLabel?: (inputValue: string) => React.ReactNode; + /** + * Controls when the "Add" row is visible. + * Receives the current input, selected values, and all options. + * Defaults to react-select's built-in logic (hidden when input matches an existing option label). + * Use cases: minimum-length gating, pattern validation, case-insensitive dedup, max-items cap. + */ + isValidNewOption?: ( + inputValue: string, + value: OptionsType, + options: OptionsType + ) => boolean; + /** + * Customizes the message shown inside the dropdown menu when no option matches + * the current input (react-select's "No options" state). Useful for surfacing + * validation/error text directly in the dropdown. Accepts a node, or a function + * receiving the current input value. + */ + validationMessage?: + | React.ReactNode + | ((obj: { inputValue: string }) => React.ReactNode); } /** @@ -97,13 +135,24 @@ export interface MultiSelectDropdownProps extends SelectDropdownCoreProps { onChange?: NamedProps['onChange']; } +/** + * Enforces that isSearchable cannot be false when isCreatable is true. + * Creatable mode requires the search input so users can type new option values. + */ +type CreatableConstraint = + | { isCreatable?: false | undefined; isSearchable?: boolean } + | { isCreatable: true; isSearchable?: true }; + /** * Union type for all SelectDropdown prop variants. - * Supports both single and multi-select modes through discriminated union. + * Supports both single and multi-select modes through discriminated union, + * intersected with CreatableConstraint to enforce isSearchable compatibility. */ -export type SelectDropdownProps = +export type SelectDropdownProps = ( | SingleSelectDropdownProps - | MultiSelectDropdownProps; + | MultiSelectDropdownProps +) & + CreatableConstraint; /** * Base interface for onChange-related props. @@ -120,9 +169,13 @@ export interface BaseOnChangeProps { /** * Props for the typed React Select component wrapper. - * Extends ReactSelectAdditionalProps with an optional ref. + * Extends ReactSelectAdditionalProps with an optional ref and creatable flag. */ -export interface TypedReactSelectProps extends ReactSelectAdditionalProps { +export interface TypedReactSelectProps + extends ReactSelectAdditionalProps, + Pick { /** Optional ref to the underlying react-select component */ selectRef?: Ref; + /** When true, renders CreatableSelect instead of ReactSelect */ + isCreatable?: boolean; } diff --git a/packages/gamut/src/Form/SelectDropdown/types/internal.ts b/packages/gamut/src/Form/SelectDropdown/types/internal.ts index 8a151e2102c..9284f1db249 100644 --- a/packages/gamut/src/Form/SelectDropdown/types/internal.ts +++ b/packages/gamut/src/Form/SelectDropdown/types/internal.ts @@ -17,7 +17,7 @@ export type InternalSelectProps = { * Ref type for programmatic focus management. * Used for managing focus on select input and remove all button. */ -export type ProgramaticFocusRef = +export type ProgrammaticFocusRef = | React.MutableRefObject | React.MutableRefObject; @@ -31,9 +31,9 @@ export interface SelectDropdownContextValueTypes { /** Function to update the currently focused value */ setCurrentFocusedValue?: React.Dispatch>; /** Ref to the select input for programmatic focus */ - selectInputRef?: ProgramaticFocusRef; + selectInputRef?: ProgrammaticFocusRef; /** Ref to the remove all button for programmatic focus */ - removeAllButtonRef?: ProgramaticFocusRef; + removeAllButtonRef?: ProgrammaticFocusRef; } /** diff --git a/packages/gamut/src/Form/SelectDropdown/types/styles.ts b/packages/gamut/src/Form/SelectDropdown/types/styles.ts index 3be7acbd824..59d3abb84d2 100644 --- a/packages/gamut/src/Form/SelectDropdown/types/styles.ts +++ b/packages/gamut/src/Form/SelectDropdown/types/styles.ts @@ -1,6 +1,6 @@ import { StyleProps } from '@codecademy/variance'; -import { conditionalBorderStates } from '../styles'; +import { conditionalBorderStates } from '../core/styles'; import { InternalInputsProps } from './component-props'; /** @@ -83,4 +83,6 @@ export type OptionState = BaseSelectComponentProps & InteractionStates & { /** Whether the option is selected */ isSelected: boolean; + /** Option data — includes __isNew__ for react-select/creatable's "Add" row */ + data?: { __isNew__?: boolean }; }; diff --git a/packages/gamut/src/Form/__tests__/SelectDropdown.test.tsx b/packages/gamut/src/Form/__tests__/SelectDropdown.test.tsx index 3b068aaaa93..90cd8bc274a 100644 --- a/packages/gamut/src/Form/__tests__/SelectDropdown.test.tsx +++ b/packages/gamut/src/Form/__tests__/SelectDropdown.test.tsx @@ -1,6 +1,7 @@ import { setupRtl } from '@codecademy/gamut-tests'; +import { fireEvent } from '@testing-library/dom'; import userEvent from '@testing-library/user-event'; -import { act } from 'react'; +import { act, useState } from 'react'; import { openDropdown, @@ -11,6 +12,59 @@ import { } from '../__fixtures__/utils'; import { SelectDropdown } from '../SelectDropdown'; +const CreatableMultiHarness = () => { + const [options, setOptions] = useState(['Apple', 'Banana']); + + return ( + + setOptions((prev) => [...prev, inputValue]) + } + /> + ); +}; + +const ControlledCreatableMultiHarness = ({ + onChange, + onCreateOption, +}: { + onChange?: jest.Mock; + onCreateOption?: jest.Mock; +}) => { + const [options, setOptions] = useState(['Apple', 'Banana']); + const [value, setValue] = useState(['Apple']); + + return ( + { + setValue(selected.map((option) => option.value)); + + if (meta.action === 'create-option' && meta.option) { + setOptions((prev) => [...prev, meta.option.value]); + } + + onChange?.(selected, meta); + }} + onCreateOption={onCreateOption} + /> + ); +}; + +const renderCreatableMulti = setupRtl(CreatableMultiHarness, {}); +const renderControlledCreatableMulti = setupRtl( + ControlledCreatableMultiHarness, + {} +); + /** There is a state pollution issue with SelectDropdown and jest which is why these are broken up into their own file. * Ticket to fix: https://skillsoftdev.atlassian.net/browse/GM-1297 */ @@ -50,359 +104,611 @@ const renderView = setupRtl(SelectDropdown, { }); describe('SelectDropdown', () => { - it('sets the id prop on the select tag', () => { - const { view } = renderView(); + describe('default', () => { + it('sets the id prop on the select tag', () => { + const { view } = renderView(); - expect(view.getByRole('combobox')).toHaveAttribute('id', 'colors'); - }); + expect(view.getByRole('combobox')).toHaveAttribute('id', 'colors'); + }); - it.each([ - ['array', selectOptions], - ['object', selectOptionsObject], - ])('renders options when options is an %s', async (_, options) => { - const { view } = renderView({ options }); + it.each([ + ['array', selectOptions], + ['object', selectOptionsObject], + ])('renders options when options is an %s', async (_, options) => { + const { view } = renderView({ options }); - await openDropdown(view); + await openDropdown(view); - view.getByText('green'); - }); + view.getByText('green'); + }); - it('renders a small dropdown when size is "small"', () => { - const { view } = renderView({ size: 'small' }); - view.getByTitle('Mini Chevron Down Icon'); - }); + it('renders a small dropdown when size is "small"', () => { + const { view } = renderView({ size: 'small' }); + view.getByTitle('Mini Chevron Down Icon'); + }); - it('renders a medium dropdown when size is "medium"', () => { - const { view } = renderView({ size: 'medium' }); - view.getByTitle('Arrow Chevron Down Icon'); - }); + it('renders a medium dropdown when size is "medium"', () => { + const { view } = renderView({ size: 'medium' }); + view.getByTitle('Arrow Chevron Down Icon'); + }); - it('renders a medium dropdown by default', () => { - const { view } = renderView(); - view.getByTitle('Arrow Chevron Down Icon'); - }); + it('renders a medium dropdown by default', () => { + const { view } = renderView(); + view.getByTitle('Arrow Chevron Down Icon'); + }); - it('renders a dropdown with the correct maxHeight when shownOptionsLimit is specified', async () => { - const { view } = renderView({ shownOptionsLimit: 4 }); + it('renders a dropdown with the correct maxHeight when shownOptionsLimit is specified', async () => { + const { view } = renderView({ shownOptionsLimit: 4 }); - await openDropdown(view); + await openDropdown(view); - expect(view.getByRole('listbox')).toHaveStyle({ maxHeight: '12rem' }); - }); - it('renders a dropdown with the correct maxHeight when shownOptionsLimit is specified + size is "small"', async () => { - const { view } = renderView({ - size: 'small', - shownOptionsLimit: 4, + expect(view.getByRole('listbox')).toHaveStyle({ maxHeight: '12rem' }); }); + it('renders a dropdown with the correct maxHeight when shownOptionsLimit is specified + size is "small"', async () => { + const { view } = renderView({ + size: 'small', + shownOptionsLimit: 4, + }); - await openDropdown(view); - - expect(view.getByRole('listbox')).toHaveStyle({ maxHeight: '8rem' }); - }); + await openDropdown(view); - it('renders a dropdown with icons', async () => { - const { view } = renderView({ options: optionsIconsArray }); + expect(view.getByRole('listbox')).toHaveStyle({ maxHeight: '8rem' }); + }); - await openDropdown(view); + it('renders a dropdown with icons', async () => { + const { view } = renderView({ options: optionsIconsArray }); - optionsIconsArray.forEach((icon) => expect(view.getByTitle(icon.label))); - }); + await openDropdown(view); - it('displays icon in selected value when option has icon', async () => { - const { view } = renderView({ - options: optionsIconsArray, - value: 'one', + optionsIconsArray.forEach((icon) => expect(view.getByTitle(icon.label))); }); - expect(view.getByTitle('Data Transfer Vertical Icon')).toBeInTheDocument(); - const selectedValueContainer = view.getByRole('combobox').closest('div'); - expect(selectedValueContainer).toHaveTextContent( - 'Data Transfer Vertical Icon' - ); - }); + it('displays icon in selected value when option has icon', async () => { + const { view } = renderView({ + options: optionsIconsArray, + value: 'one', + }); - it('function passed to onInputChanges is called on input change', async () => { - const onInputChange = jest.fn(); - const { view } = renderView({ onInputChange }); + expect( + view.getByTitle('Data Transfer Vertical Icon') + ).toBeInTheDocument(); + const selectedValueContainer = view.getByRole('combobox').closest('div'); + expect(selectedValueContainer).toHaveTextContent( + 'Data Transfer Vertical Icon' + ); + }); - await openDropdown(view); + it('function passed to onInputChanges is called on input change', async () => { + const onInputChange = jest.fn(); + const { view } = renderView({ onInputChange }); - await act(async () => { - await userEvent.click(view.getByText('red')); - }); + await openDropdown(view); - expect(onInputChange).toHaveBeenCalled(); - }); + await act(async () => { + await userEvent.click(view.getByText('red')); + }); - it('works with multiple selection', async () => { - const onChange = jest.fn(); - const { view } = renderView({ - multiple: true, - onChange, + expect(onInputChange).toHaveBeenCalled(); }); - await openDropdown(view); - await act(async () => { - await userEvent.click(view.getByText('red')); - }); + it('works with multiple selection', async () => { + const onChange = jest.fn(); + const { view } = renderView({ + multiple: true, + onChange, + }); - await openDropdown(view); - await act(async () => { - await userEvent.click(view.getByText('green')); - }); + await openDropdown(view); + await act(async () => { + await userEvent.click(view.getByText('red')); + }); - view.getByText('red'); - view.getByText('green'); + await openDropdown(view); + await act(async () => { + await userEvent.click(view.getByText('green')); + }); - expect(onChange).toHaveBeenCalledTimes(2); - expect(onChange).toHaveBeenNthCalledWith( - 1, - [ - { - label: 'red', - value: 'red', - }, - ], - { - action: 'select-option', - } - ); - expect(onChange).toHaveBeenNthCalledWith( - 2, - [ + view.getByText('red'); + view.getByText('green'); + + expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange).toHaveBeenNthCalledWith( + 1, + [ + { + label: 'red', + value: 'red', + }, + ], { - label: 'red', - value: 'red', - }, + action: 'select-option', + } + ); + expect(onChange).toHaveBeenNthCalledWith( + 2, + [ + { + label: 'red', + value: 'red', + }, + { + label: 'green', + value: 'green', + }, + ], { - label: 'green', - value: 'green', - }, - ], - { - action: 'select-option', - } - ); - }); - - it('displays abbreviations in multiselect mode', async () => { - const onChange = jest.fn(); - const { view } = renderView({ - multiple: true, - options: optionsWithAbbreviations, - onChange, + action: 'select-option', + } + ); }); - await openDropdown(view); - await act(async () => { - await userEvent.click(view.getByText('United States of America')); - }); + it('displays abbreviations in multiselect mode', async () => { + const onChange = jest.fn(); + const { view } = renderView({ + multiple: true, + options: optionsWithAbbreviations, + onChange, + }); - await openDropdown(view); - await act(async () => { - await userEvent.click(view.getByText('United Kingdom')); - }); + await openDropdown(view); + await act(async () => { + await userEvent.click(view.getByText('United States of America')); + }); - view.getByText('USA'); - view.getByText('UK'); + await openDropdown(view); + await act(async () => { + await userEvent.click(view.getByText('United Kingdom')); + }); - expect(onChange).toHaveBeenCalledTimes(2); - expect(onChange).toHaveBeenNthCalledWith( - 1, - [ - { - label: 'United States of America', - value: 'usa', - abbreviation: 'USA', - key: 'usa', - size: undefined, - }, - ], - { - action: 'select-option', - option: undefined, - } - ); - expect(onChange).toHaveBeenNthCalledWith( - 2, - [ + view.getByText('USA'); + view.getByText('UK'); + + expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange).toHaveBeenNthCalledWith( + 1, + [ + { + label: 'United States of America', + value: 'usa', + abbreviation: 'USA', + key: 'usa', + size: undefined, + }, + ], { - label: 'United States of America', - value: 'usa', - abbreviation: 'USA', - key: 'usa', - size: undefined, - }, + action: 'select-option', + option: undefined, + } + ); + expect(onChange).toHaveBeenNthCalledWith( + 2, + [ + { + label: 'United States of America', + value: 'usa', + abbreviation: 'USA', + key: 'usa', + size: undefined, + }, + { + label: 'United Kingdom', + value: 'uk', + abbreviation: 'UK', + key: 'uk', + size: undefined, + }, + ], { - label: 'United Kingdom', - value: 'uk', - abbreviation: 'UK', - key: 'uk', - size: undefined, - }, - ], - { - action: 'select-option', - option: undefined, - } - ); - }); + action: 'select-option', + option: undefined, + } + ); + }); - describe('inputProps functionality', () => { - it('should apply combobox and hidden props in non-searchable mode (default)', () => { - const { view } = renderView({ - isSearchable: false, - inputProps: { - hidden: { - 'data-form-field': 'test-field', - 'data-hidden-attr': 'hidden-value', + describe('inputProps functionality', () => { + it('should apply combobox and hidden props in non-searchable mode (default)', () => { + const { view } = renderView({ + isSearchable: false, + inputProps: { + hidden: { + 'data-form-field': 'test-field', + 'data-hidden-attr': 'hidden-value', + }, + combobox: { + 'data-testid': 'non-searchable-combobox', + 'data-custom-attr': 'custom-value', + }, }, - combobox: { - 'data-testid': 'non-searchable-combobox', - 'data-custom-attr': 'custom-value', + }); + + const comboboxInput = view.getByRole('combobox'); + expect(comboboxInput).toHaveAttribute( + 'data-testid', + 'non-searchable-combobox' + ); + expect(comboboxInput).toHaveAttribute( + 'data-custom-attr', + 'custom-value' + ); + + const hiddenInput = view.container.querySelector( + 'input[type="hidden"][data-form-field="test-field"]' + ); + expect(hiddenInput).toHaveAttribute('data-form-field', 'test-field'); + expect(hiddenInput).toHaveAttribute('data-hidden-attr', 'hidden-value'); + }); + + it('should apply combobox and hidden props in searchable mode', () => { + const { view } = renderView({ + isSearchable: true, + inputProps: { + hidden: { + 'data-form-field': 'searchable-field', + 'data-hidden-attr': 'searchable-hidden-value', + }, + combobox: { + 'data-testid': 'searchable-combobox', + 'data-custom-attr': 'searchable-custom-value', + }, }, - }, + }); + + const comboboxInput = view.getByRole('combobox'); + expect(comboboxInput).toHaveAttribute( + 'data-testid', + 'searchable-combobox' + ); + expect(comboboxInput).toHaveAttribute( + 'data-custom-attr', + 'searchable-custom-value' + ); + + const hiddenInput = view.container.querySelector( + 'input[type="hidden"][data-form-field="searchable-field"]' + ); + expect(hiddenInput).toHaveAttribute( + 'data-form-field', + 'searchable-field' + ); + expect(hiddenInput).toHaveAttribute( + 'data-hidden-attr', + 'searchable-hidden-value' + ); }); - const comboboxInput = view.getByRole('combobox'); - expect(comboboxInput).toHaveAttribute( - 'data-testid', - 'non-searchable-combobox' - ); - expect(comboboxInput).toHaveAttribute('data-custom-attr', 'custom-value'); + it('should work with only combobox props (no hidden props)', () => { + const { view } = renderView({ + isSearchable: false, + inputProps: { + combobox: { + 'data-testid': 'combobox-only', + 'data-aria-label': 'Custom combobox label', + }, + }, + }); + + const comboboxInput = view.getByRole('combobox'); + expect(comboboxInput).toHaveAttribute('data-testid', 'combobox-only'); + expect(comboboxInput).toHaveAttribute( + 'data-aria-label', + 'Custom combobox label' + ); + }); - const hiddenInput = view.container.querySelector( - 'input[type="hidden"][data-form-field="test-field"]' - ); - expect(hiddenInput).toHaveAttribute('data-form-field', 'test-field'); - expect(hiddenInput).toHaveAttribute('data-hidden-attr', 'hidden-value'); - }); + it('should work with only hidden props (no combobox props)', () => { + const { view } = renderView({ + isSearchable: true, + inputProps: { + hidden: { + 'data-form-field': 'hidden-only', + 'data-validation': 'required', + }, + }, + }); + + const comboboxInput = view.getByRole('combobox'); + // Should not have any combobox-specific attributes + expect(comboboxInput).not.toHaveAttribute('data-form-field'); + expect(comboboxInput).not.toHaveAttribute('data-validation'); + + const hiddenInput = view.container.querySelector( + 'input[type="hidden"][data-form-field="hidden-only"]' + ); + expect(hiddenInput).toHaveAttribute('data-form-field', 'hidden-only'); + expect(hiddenInput).toHaveAttribute('data-validation', 'required'); + }); - it('should apply combobox and hidden props in searchable mode', () => { - const { view } = renderView({ - isSearchable: true, - inputProps: { - hidden: { - 'data-form-field': 'searchable-field', - 'data-hidden-attr': 'searchable-hidden-value', + it('should handle multiple data attributes in combobox props', () => { + const { view } = renderView({ + isSearchable: false, + inputProps: { + combobox: { + 'data-testid': 'multi-attr-test', + 'data-cy': 'combobox-element', + 'data-analytics': 'user-interaction', + 'data-tracking': 'form-field', + 'aria-describedby': 'help-text', + }, }, - combobox: { - 'data-testid': 'searchable-combobox', - 'data-custom-attr': 'searchable-custom-value', + }); + + const comboboxInput = view.getByRole('combobox'); + expect(comboboxInput).toHaveAttribute('data-testid', 'multi-attr-test'); + expect(comboboxInput).toHaveAttribute('data-cy', 'combobox-element'); + expect(comboboxInput).toHaveAttribute( + 'data-analytics', + 'user-interaction' + ); + expect(comboboxInput).toHaveAttribute('data-tracking', 'form-field'); + expect(comboboxInput).toHaveAttribute('aria-describedby', 'help-text'); + }); + + it('should handle boolean and number values in combobox props', () => { + const { view } = renderView({ + isSearchable: true, + inputProps: { + combobox: { + 'data-testid': 'type-test', + 'data-boolean-true': true, + 'data-boolean-false': false, + 'data-number': 42, + 'data-zero': 0, + }, }, - }, + }); + + const comboboxInput = view.getByRole('combobox'); + expect(comboboxInput).toHaveAttribute('data-testid', 'type-test'); + expect(comboboxInput).toHaveAttribute('data-boolean-true', 'true'); + expect(comboboxInput).toHaveAttribute('data-boolean-false', 'false'); + expect(comboboxInput).toHaveAttribute('data-number', '42'); + expect(comboboxInput).toHaveAttribute('data-zero', '0'); }); + }); - const comboboxInput = view.getByRole('combobox'); - expect(comboboxInput).toHaveAttribute( - 'data-testid', - 'searchable-combobox' - ); - expect(comboboxInput).toHaveAttribute( - 'data-custom-attr', - 'searchable-custom-value' - ); + describe('validationMessage', () => { + it('renders custom node text in place of the default "No options" state', async () => { + const { view } = renderView({ + options: [], + validationMessage: 'No fruits available', + }); - const hiddenInput = view.container.querySelector( - 'input[type="hidden"][data-form-field="searchable-field"]' - ); - expect(hiddenInput).toHaveAttribute( - 'data-form-field', - 'searchable-field' - ); - expect(hiddenInput).toHaveAttribute( - 'data-hidden-attr', - 'searchable-hidden-value' - ); + await openDropdown(view); + + expect(view.getByText('No fruits available')).toBeInTheDocument(); + expect(view.queryByText('No options')).not.toBeInTheDocument(); + }); + }); + }); + + describe('isCreatable', () => { + it('shows the "Add" row when input does not match any existing option', async () => { + const { view } = renderView({ isCreatable: true }); + + await act(async () => { + await userEvent.type(view.getByRole('combobox'), 'purple'); + }); + + expect(view.getByText('Add "purple"')).toBeInTheDocument(); }); - it('should work with only combobox props (no hidden props)', () => { - const { view } = renderView({ - isSearchable: false, - inputProps: { - combobox: { - 'data-testid': 'combobox-only', - 'data-aria-label': 'Custom combobox label', - }, - }, + it('does not show the "Add" row when input matches an existing option', async () => { + const { view } = renderView({ isCreatable: true }); + + await act(async () => { + await userEvent.type(view.getByRole('combobox'), 'red'); }); - const comboboxInput = view.getByRole('combobox'); - expect(comboboxInput).toHaveAttribute('data-testid', 'combobox-only'); - expect(comboboxInput).toHaveAttribute( - 'data-aria-label', - 'Custom combobox label' - ); + expect(view.queryByText('Add "red"')).not.toBeInTheDocument(); }); - it('should work with only hidden props (no combobox props)', () => { + it('fires onCreateOption with the typed value when the "Add" row is selected', async () => { + const onCreateOption = jest.fn(); + const { view } = renderView({ isCreatable: true, onCreateOption }); + + await act(async () => { + await userEvent.type(view.getByRole('combobox'), 'purple'); + }); + + await act(async () => { + await userEvent.click(view.getByText('Add "purple"')); + }); + + expect(onCreateOption).toHaveBeenCalledWith('purple'); + }); + + it('fires onChange with create-option when the "Add" row is selected in multi mode', async () => { + const onChange = jest.fn(); const { view } = renderView({ - isSearchable: true, - inputProps: { - hidden: { - 'data-form-field': 'hidden-only', - 'data-validation': 'required', - }, - }, + isCreatable: true, + multiple: true, + onChange, + }); + + await openDropdown(view); + await act(async () => { + await userEvent.click(view.getByText('red')); }); - const comboboxInput = view.getByRole('combobox'); - // Should not have any combobox-specific attributes - expect(comboboxInput).not.toHaveAttribute('data-form-field'); - expect(comboboxInput).not.toHaveAttribute('data-validation'); + const combobox = view.getByRole('combobox'); + + await act(async () => { + await userEvent.type(combobox, 'purple'); + }); - const hiddenInput = view.container.querySelector( - 'input[type="hidden"][data-form-field="hidden-only"]' + await act(async () => { + await userEvent.click(view.getByText('Add "purple"')); + }); + + expect(onChange).toHaveBeenLastCalledWith( + [ + { label: 'red', value: 'red' }, + { label: 'purple', value: 'purple', __isNew__: true }, + ], + expect.objectContaining({ + action: 'create-option', + option: expect.objectContaining({ value: 'purple' }), + }) ); - expect(hiddenInput).toHaveAttribute('data-form-field', 'hidden-only'); - expect(hiddenInput).toHaveAttribute('data-validation', 'required'); }); - it('should handle multiple data attributes in combobox props', () => { + it('respects a custom formatCreateLabel', async () => { const { view } = renderView({ - isSearchable: false, - inputProps: { - combobox: { - 'data-testid': 'multi-attr-test', - 'data-cy': 'combobox-element', - 'data-analytics': 'user-interaction', - 'data-tracking': 'form-field', - 'aria-describedby': 'help-text', - }, - }, + isCreatable: true, + formatCreateLabel: (v: string) => `Create tag: "${v}"`, }); - const comboboxInput = view.getByRole('combobox'); - expect(comboboxInput).toHaveAttribute('data-testid', 'multi-attr-test'); - expect(comboboxInput).toHaveAttribute('data-cy', 'combobox-element'); - expect(comboboxInput).toHaveAttribute( - 'data-analytics', - 'user-interaction' - ); - expect(comboboxInput).toHaveAttribute('data-tracking', 'form-field'); - expect(comboboxInput).toHaveAttribute('aria-describedby', 'help-text'); + await act(async () => { + await userEvent.type(view.getByRole('combobox'), 'purple'); + }); + + expect(view.getByText('Create tag: "purple"')).toBeInTheDocument(); }); - it('should handle boolean and number values in combobox props', () => { + it('hides the "Add" row when isValidNewOption returns false', async () => { const { view } = renderView({ - isSearchable: true, - inputProps: { - combobox: { - 'data-testid': 'type-test', - 'data-boolean-true': true, - 'data-boolean-false': false, - 'data-number': 42, - 'data-zero': 0, - }, - }, + isCreatable: true, + isValidNewOption: () => false, + }); + + await act(async () => { + await userEvent.type(view.getByRole('combobox'), 'anything'); + }); + + expect(view.queryByText('Add "anything"')).not.toBeInTheDocument(); + }); + + it('clears the typed text when the input blurs', async () => { + const { view } = renderView({ isCreatable: true }); + + const combobox = view.getByRole('combobox'); + + await act(async () => { + await userEvent.type(combobox, 'pur'); + }); + + await act(async () => { + fireEvent.focusOut(combobox); + }); + + expect(combobox).toHaveValue(''); + }); + + it('forwards onInputChange to the consumer when the input blurs', async () => { + const onInputChange = jest.fn(); + const { view } = renderView({ isCreatable: true, onInputChange }); + + const combobox = view.getByRole('combobox'); + + await act(async () => { + await userEvent.type(combobox, 'pur'); + }); + + onInputChange.mockClear(); + + await act(async () => { + fireEvent.focusOut(combobox); }); - const comboboxInput = view.getByRole('combobox'); - expect(comboboxInput).toHaveAttribute('data-testid', 'type-test'); - expect(comboboxInput).toHaveAttribute('data-boolean-true', 'true'); - expect(comboboxInput).toHaveAttribute('data-boolean-false', 'false'); - expect(comboboxInput).toHaveAttribute('data-number', '42'); - expect(comboboxInput).toHaveAttribute('data-zero', '0'); + expect(onInputChange).toHaveBeenCalledWith('', { + action: 'input-blur', + prevInputValue: 'pur', + }); + }); + + it('clears the typed text after an option is created', async () => { + const { view } = renderView({ isCreatable: true }); + + const combobox = view.getByRole('combobox'); + + await act(async () => { + await userEvent.type(combobox, 'purple'); + }); + + await act(async () => { + await userEvent.click(view.getByText('Add "purple"')); + }); + + expect(combobox).toHaveValue(''); + }); + + it('keeps existing multi selections when a new option is created', async () => { + const { view } = renderCreatableMulti(); + + await openDropdown(view); + await act(async () => { + await userEvent.click(view.getByText('Apple')); + }); + + await openDropdown(view); + await act(async () => { + await userEvent.click(view.getByText('Banana')); + }); + + const combobox = view.getByRole('combobox'); + + await act(async () => { + await userEvent.type(combobox, 'Cherry'); + }); + + await act(async () => { + await userEvent.click(view.getByText('Add "Cherry"')); + }); + + expect(view.getByText('Apple')).toBeInTheDocument(); + expect(view.getByText('Banana')).toBeInTheDocument(); + expect(view.getByText('Cherry')).toBeInTheDocument(); + }); + + it('keeps controlled multi selections when a new option is created', async () => { + const onChange = jest.fn(); + const { view } = renderControlledCreatableMulti({ onChange }); + + await openDropdown(view); + await act(async () => { + await userEvent.click(view.getByText('Banana')); + }); + + const combobox = view.getByRole('combobox'); + + await act(async () => { + await userEvent.type(combobox, 'Cherry'); + }); + + await act(async () => { + await userEvent.click(view.getByText('Add "Cherry"')); + }); + + expect(view.getByText('Apple')).toBeInTheDocument(); + expect(view.getByText('Banana')).toBeInTheDocument(); + expect(view.getByText('Cherry')).toBeInTheDocument(); + + expect(onChange).toHaveBeenLastCalledWith( + expect.arrayContaining([ + expect.objectContaining({ value: 'Apple' }), + expect.objectContaining({ value: 'Banana' }), + expect.objectContaining({ value: 'Cherry' }), + ]), + expect.objectContaining({ action: 'create-option' }) + ); + }); + + describe('validationMessage', () => { + it('supports a function that receives the current input value', async () => { + const { view } = renderView({ + isCreatable: true, + isValidNewOption: () => false, + options: [], + validationMessage: ({ inputValue }: { inputValue: string }) => + `No match for "${inputValue}"`, + }); + + await act(async () => { + await userEvent.type(view.getByRole('combobox'), 'kiwi'); + }); + + expect(view.getByText('No match for "kiwi"')).toBeInTheDocument(); + }); }); }); }); diff --git a/packages/gamut/src/Form/__tests__/__snapshots__/utils.test.tsx.snap b/packages/gamut/src/Form/__tests__/__snapshots__/utils.test.tsx.snap index 2690797df50..6db6a89db06 100644 --- a/packages/gamut/src/Form/__tests__/__snapshots__/utils.test.tsx.snap +++ b/packages/gamut/src/Form/__tests__/__snapshots__/utils.test.tsx.snap @@ -2,39 +2,63 @@ exports[`parseSelectOptions creates an option list 1`] = ` [ - , - , + { + "$$typeof": Symbol(react.transitional.element), + "_owner": null, + "_store": {}, + "key": "test-val", + "props": { + "children": "Value", + "data-testid": "test-val", + "label": "Value", + "value": "val", + }, + "type": "option", + }, + { + "$$typeof": Symbol(react.transitional.element), + "_owner": null, + "_store": {}, + "key": "test-val2", + "props": { + "children": "Value 2", + "data-testid": "test-val2", + "label": "Value 2", + "value": "val2", + }, + "type": "option", + }, ] `; exports[`parseSelectOptions creates an option list 2`] = ` [ - , - , + { + "$$typeof": Symbol(react.transitional.element), + "_owner": null, + "_store": {}, + "key": "test-val", + "props": { + "children": "val", + "data-testid": "test-val", + "label": "val", + "value": "val", + }, + "type": "option", + }, + { + "$$typeof": Symbol(react.transitional.element), + "_owner": null, + "_store": {}, + "key": "test-val2", + "props": { + "children": "val2", + "data-testid": "test-val2", + "label": "val2", + "value": "val2", + }, + "type": "option", + }, ] `; diff --git a/packages/gamut/src/Form/styles/index.ts b/packages/gamut/src/Form/styles/index.ts index 51d46d8fac2..758c4a1fd65 100644 --- a/packages/gamut/src/Form/styles/index.ts +++ b/packages/gamut/src/Form/styles/index.ts @@ -1,4 +1,4 @@ export * from './shared-system-props'; export * from './Checkbox-styles'; export * from './Radio-styles'; -export * from '../SelectDropdown/styles'; +export * from '../SelectDropdown/core/styles'; diff --git a/packages/gamut/src/GridForm/GridForm.tsx b/packages/gamut/src/GridForm/GridForm.tsx index 001e2ace889..d1d74d6df17 100644 --- a/packages/gamut/src/GridForm/GridForm.tsx +++ b/packages/gamut/src/GridForm/GridForm.tsx @@ -137,7 +137,7 @@ export function GridForm>({ const showRequiredText = hideRequiredText ? false : !hasComputedSoloField; return ( - + } display="flex" flexDirection="column" diff --git a/packages/gamut/src/Menu/MenuItem.tsx b/packages/gamut/src/Menu/MenuItem.tsx index c08251a7f0a..d8ba9af3fd9 100644 --- a/packages/gamut/src/Menu/MenuItem.tsx +++ b/packages/gamut/src/Menu/MenuItem.tsx @@ -4,7 +4,7 @@ import { ComponentProps, forwardRef, MouseEventHandler, - MutableRefObject, + Ref, useId, } from 'react'; @@ -60,6 +60,18 @@ interface MenuTextItem extends HTMLProps, ForwardListItemProps { type MenuItemTypes = MenuItemIconOnly | MenuTextItem; +type MenuItemRefElement = HTMLLIElement | HTMLAnchorElement | HTMLButtonElement; + +/** + * Narrows the forwarded ref union to a specific element type for the current render branch. + * MenuItem renders exactly one of li, a, or button per call, so the ref is forwarded to that element. + */ +function narrowMenuItemRef( + ref: React.Ref +): Ref { + return ref as Ref; +} + export const MenuItem = forwardRef< HTMLLIElement | HTMLAnchorElement | HTMLButtonElement, MenuItemTypes @@ -134,15 +146,13 @@ export const MenuItem = forwardRef< ); if (listItemType === 'link' && !disabled) { - const linkRef = ref as MutableRefObject; - return ( (ref)} target={target} > {content} @@ -153,7 +163,6 @@ export const MenuItem = forwardRef< } if (listItemType === 'button' || (listItemType === 'link' && disabled)) { - const buttonRef = ref as MutableRefObject; const handleClick: MouseEventHandler = disabled ? () => null : (props.onClick as any as MouseEventHandler); @@ -163,7 +172,7 @@ export const MenuItem = forwardRef< (ref)} onClick={handleClick} > {content} @@ -173,11 +182,12 @@ export const MenuItem = forwardRef< ); } - const liRef = ref as MutableRefObject; - return ( // These are non-interactive and will never have tooltips (nor should they). - + (ref)} + > {content} ); diff --git a/packages/gamut/src/Menu/__tests__/Menu.test.tsx b/packages/gamut/src/Menu/__tests__/Menu.test.tsx index 883c39d1604..bd8e0e50fa8 100644 --- a/packages/gamut/src/Menu/__tests__/Menu.test.tsx +++ b/packages/gamut/src/Menu/__tests__/Menu.test.tsx @@ -1,7 +1,8 @@ import { MultipleUsersIcon } from '@codecademy/gamut-icons'; import { setupRtl } from '@codecademy/gamut-tests'; -import { screen } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import React from 'react'; import { Menu } from '../Menu'; import { MenuItem } from '../MenuItem'; @@ -32,6 +33,30 @@ describe('Menu', () => { screen.getByRole('link'); expect(screen.queryByRole('menuitem')).toBeNull(); }); + + it('forwards ref to the link when MenuItem has href', () => { + const ref = React.createRef(); + render( + + + Cool Town + + + ); + expect(ref.current).toBeInstanceOf(HTMLAnchorElement); + }); + + it('forwards ref to the button when MenuItem has onClick', () => { + const ref = React.createRef(); + render( + + null}> + Cool Town + + + ); + expect(ref.current).toBeInstanceOf(HTMLButtonElement); + }); it('renders MenuItems with onClicks as buttons within a li', () => { renderView({ children: null}>Cool Town, diff --git a/packages/gamut/src/Modals/Dialog.tsx b/packages/gamut/src/Modals/Dialog.tsx index 7ad3acd7d09..ea8fe3698b2 100644 --- a/packages/gamut/src/Modals/Dialog.tsx +++ b/packages/gamut/src/Modals/Dialog.tsx @@ -1,11 +1,12 @@ import { MiniDeleteIcon } from '@codecademy/gamut-icons'; -import { ComponentProps } from 'react'; +import { ComponentProps, useId } from 'react'; import * as React from 'react'; import { Box } from '../Box'; import { FillButton, IconButton, TextButton } from '../Button'; import { Overlay } from '../Overlay'; import { Text } from '../Typography'; +import { isNullish } from '../utils/nullish'; import { ModalContainer, ModalContainerProps } from './elements'; import { ImageContainer } from './ImageContainer'; import { CloseButtonProps, ModalBaseProps } from './types'; @@ -46,6 +47,8 @@ export const Dialog: React.FC = ({ size = 'small', ...rest }) => { + const titleId = useId(); + const onConfirm: DialogButtonProps['onClick'] = ( e: React.MouseEvent ) => { @@ -64,8 +67,8 @@ export const Dialog: React.FC = ({ void} {...rest}> = ({ size={size} tabIndex={-1} > - + {title} {!hideCloseButton && ( diff --git a/packages/gamut/src/Modals/Modal.tsx b/packages/gamut/src/Modals/Modal.tsx index dc3db860b29..550d8981ecb 100644 --- a/packages/gamut/src/Modals/Modal.tsx +++ b/packages/gamut/src/Modals/Modal.tsx @@ -1,5 +1,5 @@ import { MiniDeleteIcon } from '@codecademy/gamut-icons'; -import { ComponentProps, useState } from 'react'; +import { ComponentProps, useId, useState } from 'react'; import * as React from 'react'; import { Box } from '../Box'; @@ -95,11 +95,14 @@ export const Modal: React.FC = ({ views, ...rest }) => { + const titleId = useId(); const [currentView, setCurrentView] = useState(0); const view = views?.[currentView]; const image = (view?.image || rest?.image) ?? null; const titleText = title || views?.[currentView].title; + const needsLabelledBy = titleText && !ariaLabel; + return ( = ({ = ({ as={headingLevel} fontSize={20} gridArea="title" + id={titleId} lineHeight="base" > {titleText} diff --git a/packages/gamut/src/Pagination/utils.tsx b/packages/gamut/src/Pagination/utils.tsx index 930c3e9cdbe..8f16cbf92ae 100644 --- a/packages/gamut/src/Pagination/utils.tsx +++ b/packages/gamut/src/Pagination/utils.tsx @@ -1,3 +1,4 @@ +import { timingValues } from '@codecademy/gamut-styles'; import { AnimatePresence, motion } from 'framer-motion'; import { useEffect, useRef } from 'react'; import * as React from 'react'; @@ -5,6 +6,8 @@ import * as React from 'react'; import { BaseEllipsisButton } from './EllipsisButton'; import { PaginationButton } from './PaginationButton'; +const FADE_DURATION_SECONDS = timingValues.base / 1000; + type PaginationUtils = { chapterSize: number; currentPage: number; @@ -76,7 +79,7 @@ export const wrapWithSlideAnimation = ( ? 'shown' : 'hidden' } - transition={{ duration: 0.3 }} + transition={{ duration: FADE_DURATION_SECONDS }} variants={slideAnimationVariants} > @@ -98,6 +101,13 @@ const fadeAnimationVariants = { }, }; +const fadeTransition = { + duration: FADE_DURATION_SECONDS, + ease: 'easeOut' as const, + transitionStart: { visibility: 'visible' as const }, + transitionEnd: { visibility: 'hidden' as const }, +}; + export const createAnimatedFadeButton = ( WrappedComponent: typeof PaginationButton ) => { @@ -110,12 +120,7 @@ export const createAnimatedFadeButton = ( } disabled={props.showButton === 'hidden'} initial={false} - transition={{ - transitionStart: { visibility: 'visible' }, - duration: 0.3, - ease: [0.04, 0.62, 0.23, 0.98], - transitionEnd: { visibility: 'hidden' }, - }} + transition={fadeTransition} variants={fadeAnimationVariants} {...props} /> diff --git a/packages/gamut/src/Popover/Popover.tsx b/packages/gamut/src/Popover/Popover.tsx index 9a1f0e0f8d4..be8421842cb 100755 --- a/packages/gamut/src/Popover/Popover.tsx +++ b/packages/gamut/src/Popover/Popover.tsx @@ -1,10 +1,10 @@ import { useElementDir, useLogicalProperties } from '@codecademy/gamut-styles'; -import type { RefObject } from 'react'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useWindowScroll, useWindowSize } from 'react-use'; import { FocusTrap } from '../FocusTrap'; import { + getRefElement, useResizingParentEffect, useScrollingParentsEffect, } from '../PopoverContainer/hooks'; @@ -54,7 +54,7 @@ export const Popover: React.FC = ({ const popoverRef = useRef(null); const logicalPropsEnabled = useLogicalProperties(); - const isRtl = useElementDir(targetRef as RefObject) === 'rtl'; + const isRtl = useElementDir(targetRef) === 'rtl'; // This only needs to resolve the positioning - the beak uses logical properties so will automatically mirror in RTL. const resolvedSideAlign = useMemo(() => { @@ -152,12 +152,12 @@ export const Popover: React.FC = ({ ]); useEffect(() => { - setTargetRect(targetRef?.current?.getBoundingClientRect()); + setTargetRect(getRefElement(targetRef)?.getBoundingClientRect()); }, [targetRef, isOpen, width, height, x, y]); const updateTargetPosition = useCallback( (rect?: DOMRect) => { - const target = targetRef?.current; + const target = getRefElement(targetRef); if (!target) return; const newRect = rect || target.getBoundingClientRect(); @@ -167,7 +167,6 @@ export const Popover: React.FC = ({ ); useScrollingParentsEffect(targetRef, updateTargetPosition); - useResizingParentEffect(targetRef, setTargetRect); useEffect(() => { @@ -187,7 +186,7 @@ export const Popover: React.FC = ({ const handleClickOutside = useCallback( (e: MouseEvent) => { const target = e.target as Node; - const targetElement = targetRef.current; + const targetElement = getRefElement(targetRef); if (!targetElement) return; @@ -201,7 +200,7 @@ export const Popover: React.FC = ({ }, [onRequestClose, targetRef] ); - if ((!isOpen || !targetRef) && !animation) return null; + if ((!isOpen || !targetRef.current) && !animation) return null; const alignment = (variant === 'primary' || beak) && beak !== 'center' ? 'aligned' @@ -214,7 +213,9 @@ export const Popover: React.FC = ({ data-floating="popover" data-testid="popover-content-container" position={position} - {...(popoverContainerRef ? { ref: popoverContainerRef } : {})} + {...(popoverContainerRef + ? { ref: popoverContainerRef as React.Ref } + : {})} role={role} // eslint-disable-next-line gamut/no-inline-style style={getPopoverPosition()} @@ -223,7 +224,7 @@ export const Popover: React.FC = ({ } variant={variant} widthRestricted={widthRestricted} > @@ -253,7 +254,10 @@ export const Popover: React.FC = ({ ); return ( - + {skipFocusTrap ? ( <>{contents} ) : ( diff --git a/packages/gamut/src/Popover/__tests__/Popover.test.tsx b/packages/gamut/src/Popover/__tests__/Popover.test.tsx index 884d956a7c6..a6881af6069 100644 --- a/packages/gamut/src/Popover/__tests__/Popover.test.tsx +++ b/packages/gamut/src/Popover/__tests__/Popover.test.tsx @@ -2,7 +2,8 @@ import { CheckerDense } from '@codecademy/gamut-patterns'; import { theme } from '@codecademy/gamut-styles'; import { setupRtl } from '@codecademy/gamut-tests'; import { ThemeProvider } from '@emotion/react'; -import { fireEvent, waitFor } from '@testing-library/react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; import { Popover, PopoverProps } from '..'; @@ -137,6 +138,23 @@ describe('Popover', () => { expect(popoverIsRendered(view)).toBeTruthy(); }); + it('accepts targetRef from useRef and renders when open', () => { + const PopoverWithUseRefTarget = () => { + const targetRef = React.useRef(null); + return ( + +
+ +
Content
+
+
outside
+ + ); + }; + render(); + expect(screen.getByTestId('popover-content')).toBeInTheDocument(); + }); + it('triggers onRequestClose callback when clicking outside', () => { const onRequestClose = jest.fn(); const { view } = renderView({ diff --git a/packages/gamut/src/Popover/types.tsx b/packages/gamut/src/Popover/types.tsx index 5495da8329d..6c931a6e284 100755 --- a/packages/gamut/src/Popover/types.tsx +++ b/packages/gamut/src/Popover/types.tsx @@ -1,5 +1,5 @@ import { PatternProps } from '@codecademy/gamut-patterns'; -import { HTMLAttributes } from 'react'; +import { HTMLAttributes, type RefObject } from 'react'; import { PopoverVariants } from './elements'; @@ -104,17 +104,14 @@ export type PopoverProps = PopoverBaseProps & /** * The target element around which the popover will be positioned. + * Only ref objects (e.g. from useRef) are supported at runtime; RefCallback is not. */ - targetRef: React.RefObject< - Pick - >; + targetRef: RefObject; /** * The PopoverContainer which contents will be rendered into. */ - popoverContainerRef?: - | React.RefObject - | React.RefCallback; + popoverContainerRef?: React.Ref; /** * Whether to add width restrictions to Popover. diff --git a/packages/gamut/src/PopoverContainer/PopoverContainer.tsx b/packages/gamut/src/PopoverContainer/PopoverContainer.tsx index c93f1965bc8..a60fa7546e7 100644 --- a/packages/gamut/src/PopoverContainer/PopoverContainer.tsx +++ b/packages/gamut/src/PopoverContainer/PopoverContainer.tsx @@ -8,11 +8,13 @@ import { useWindowScroll, useWindowSize } from 'react-use'; import { BodyPortal } from '../BodyPortal'; import { FocusTrap } from '../FocusTrap'; import { + getRefElement, + getTargetAsElement, useResizingParentEffect, useScrollingParents, useScrollingParentsEffect, } from './hooks'; -import { ContainerState, PopoverContainerProps } from './types'; +import { ContainerState, PopoverContainerProps, TargetRef } from './types'; import { getContainers, getPosition, isOutOfView } from './utils'; const PopoverContent = styled.div( @@ -90,20 +92,22 @@ export const PopoverContainer: React.FC = ({ }, [parent, x, y, offset, alignment, invertAxis, isRtl]); useEffect(() => { - const target = targetRef?.current; + const target = getRefElement(targetRef); if (!target) return; - setContainers(getContainers(target, inline, { x: winX, y: winY })); + setContainers( + getContainers(target as TargetRef, inline, { x: winX, y: winY }) + ); }, [targetRef, inline, winW, winH, winX, winY, targetRect]); // Update target rectangle when window size/scroll changes useEffect(() => { - setTargetRect(targetRef?.current?.getBoundingClientRect()); + setTargetRect(getRefElement(targetRef)?.getBoundingClientRect()); }, [targetRef, isOpen, winW, winH, winX, winY]); // Update target rectangle when parent size/scroll changes const updateTargetPosition = useCallback( (rect?: DOMRect) => { - const target = targetRef?.current; + const target = getRefElement(targetRef); if (!target) return; const newRect = rect || target.getBoundingClientRect(); @@ -115,14 +119,16 @@ export const PopoverContainer: React.FC = ({ window.pageYOffset || document.documentElement.scrollTop; setContainers( - getContainers(target, inline, { x: currentScrollX, y: currentScrollY }) + getContainers(target as TargetRef, inline, { + x: currentScrollX, + y: currentScrollY, + }) ); }, [targetRef, inline] ); useScrollingParentsEffect(targetRef, updateTargetPosition); - useResizingParentEffect(targetRef, setTargetRect); // Handle closeOnViewportExit with cached scrolling parents for performance @@ -134,7 +140,7 @@ export const PopoverContainer: React.FC = ({ const isOut = isOutOfView( rect, - targetRef?.current as HTMLElement, + getTargetAsElement(getRefElement(targetRef)) ?? undefined, scrollingParents ); @@ -158,7 +164,7 @@ export const PopoverContainer: React.FC = ({ const handleClickOutside = useCallback( (e: MouseEvent | TouchEvent) => { const target = e.target as Node; - const targetElement = targetRef.current; + const targetElement = getRefElement(targetRef); if (!targetElement) return; if (targetElement.contains(target)) return; @@ -177,7 +183,7 @@ export const PopoverContainer: React.FC = ({ const handleGlobalClickOutside = useCallback( (e: MouseEvent) => { const target = e.target as Node; - const targetElement = targetRef.current; + const targetElement = getRefElement(targetRef); if (!targetElement || !isOpen) return; @@ -229,7 +235,7 @@ export const PopoverContainer: React.FC = ({ } }, [isOpen, handleGlobalClickOutside]); - if (!isOpen || !targetRef) return null; + if (!isOpen || !targetRef.current) return null; const { children, diff --git a/packages/gamut/src/PopoverContainer/__tests__/PopoverContainer.test.tsx b/packages/gamut/src/PopoverContainer/__tests__/PopoverContainer.test.tsx index 52c5439315e..47203b9be71 100644 --- a/packages/gamut/src/PopoverContainer/__tests__/PopoverContainer.test.tsx +++ b/packages/gamut/src/PopoverContainer/__tests__/PopoverContainer.test.tsx @@ -1,5 +1,6 @@ import { MockGamutProvider, setupRtl } from '@codecademy/gamut-tests'; import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import React from 'react'; import { PopoverContainer } from '..'; import { PopoverContainerProps, TargetRef } from '../types'; @@ -95,6 +96,23 @@ describe('Popover', () => { expect(popoverIsRendered()).toBeTruthy(); }); + it('accepts targetRef from useRef and renders when open', () => { + const ContainerWithUseRefTarget = () => { + const targetRef = React.useRef(null); + return ( + +
+ +
Content
+
+
outside
+ + ); + }; + render(); + expect(screen.getByTestId('popover-content')).toBeInTheDocument(); + }); + it('triggers onRequestClose callback when clicking outside', () => { const onRequestClose = jest.fn(); renderView({ diff --git a/packages/gamut/src/PopoverContainer/__tests__/hooks.test.tsx b/packages/gamut/src/PopoverContainer/__tests__/hooks.test.tsx new file mode 100644 index 00000000000..eb56581b968 --- /dev/null +++ b/packages/gamut/src/PopoverContainer/__tests__/hooks.test.tsx @@ -0,0 +1,53 @@ +import { cleanup, render, screen } from '@testing-library/react'; +import { useLayoutEffect, useRef, useState } from 'react'; + +import { Box } from '../../Box'; +import { useScrollingParents } from '../hooks'; + +/** + * Reacts attach after paint; a render that mounts the ref target still sees + * `ref.current === null` in that pass. Real callers often get a follow-up render + * (e.g. targetRect / isOpen) once the ref is set — the layout effect simulates + * that so the hook is evaluated again with a populated ref. + */ +const ScrollingParentsFixture = ({ showTarget }: { showTarget: boolean }) => { + const ref = useRef(null); + const [, setPostAttach] = useState(0); + const parents = useScrollingParents(ref); + + useLayoutEffect(() => { + if (showTarget) { + setPostAttach((n) => n + 1); + } + }, [showTarget]); + + return ( + + + {showTarget &&
} + {parents.length} + + ); +}; + +describe('useScrollingParents', () => { + afterEach(cleanup); + + it('recomputes when the ref target is attached after the first render', () => { + const { rerender } = render(); + expect(screen.getByTestId('parent-count').textContent).toBe('0'); + + rerender(); + const count = Number.parseInt( + screen.getByTestId('parent-count').textContent ?? '', + 10 + ); + expect(count).toBeGreaterThan(0); + }); +}); diff --git a/packages/gamut/src/PopoverContainer/hooks.ts b/packages/gamut/src/PopoverContainer/hooks.ts index 295591da7cd..7a20d6b838e 100644 --- a/packages/gamut/src/PopoverContainer/hooks.ts +++ b/packages/gamut/src/PopoverContainer/hooks.ts @@ -1,28 +1,79 @@ -import { useEffect, useMemo } from 'react'; +import { + type RefObject, + useEffect, + useLayoutEffect, + useMemo, + useState, +} from 'react'; +import { isNullish } from '../utils/nullish'; import { findAllAdditionalScrollingParents, findResizingParent } from './utils'; +/** + * Minimal element shape required for popover positioning. + * Accepts both HTMLElement and TargetRef so Popover and PopoverContainer can share hooks. + */ +export interface PopoverTargetElement { + getBoundingClientRect(): DOMRect; + contains(other: Node | null): boolean; +} + +/** Resolves ref object to current element; returns null when unset. */ +export function getRefElement( + ref: RefObject +): PopoverTargetElement | null { + if (isNullish(ref)) return null; + return ref.current; +} + +/** Casts minimal target to HTMLElement for utils that need full DOM (e.g. parentElement). */ +export function getTargetAsElement( + target: PopoverTargetElement | null +): HTMLElement | null { + return target as HTMLElement | null; +} + +/** + * Syncs ref.current to React state after each commit so hooks can depend on the + * resolved element when ref object identity is stable but .current updates. + */ +function useResolvedRefTarget( + targetRef: RefObject +): PopoverTargetElement | null { + const [resolved, setResolved] = useState(() => + getRefElement(targetRef) + ); + + // ref.current updates do not change targetRef identity; run after every commit + // to sync. Functional setState bails out when the resolved node is unchanged. + // eslint-disable-next-line react-hooks/exhaustive-deps -- intentional post-commit ref sync + useLayoutEffect(() => { + const el = getRefElement(targetRef); + setResolved((prev) => (prev === el ? prev : el)); + }); + + return resolved; +} + export const useScrollingParentsEffect = ( - targetRef: React.RefObject< - Pick - >, + targetRef: RefObject, setTargetRect: (rect: DOMRect | undefined) => void ) => { + const resolvedTarget = useResolvedRefTarget(targetRef); + useEffect(() => { - if (!targetRef.current) { - return; - } + if (!resolvedTarget) return; - const target = targetRef.current as unknown as HTMLElement; - const scrollingParents = findAllAdditionalScrollingParents(target); + const scrollingParents = findAllAdditionalScrollingParents( + getTargetAsElement(resolvedTarget)! + ); const updatePosition = () => { - setTargetRect(targetRef?.current?.getBoundingClientRect()); + setTargetRect(resolvedTarget.getBoundingClientRect()); }; const cleanup: (() => void)[] = []; - // Add listeners to all scrolling parents (window scroll handled by useWindowScroll) scrollingParents.forEach((parent) => { if (parent.addEventListener) { parent.addEventListener('scroll', updatePosition, { passive: true }); @@ -35,48 +86,45 @@ export const useScrollingParentsEffect = ( return () => { cleanup.forEach((fn) => fn()); }; - }, [targetRef, setTargetRect]); + }, [resolvedTarget, setTargetRect]); }; export const useResizingParentEffect = ( - targetRef: React.RefObject< - Pick - >, + targetRef: RefObject, setTargetRect: (rect: DOMRect | undefined) => void ) => { + const resolvedTarget = useResolvedRefTarget(targetRef); + useEffect(() => { - // handles movement of target within a clipped container e.g. Drawer - if (!targetRef.current || typeof ResizeObserver === 'undefined') { - return; - } + if (!resolvedTarget || typeof ResizeObserver === 'undefined') return; + const resizingParent = findResizingParent( - targetRef.current as unknown as HTMLElement + getTargetAsElement(resolvedTarget)! ); - if (!resizingParent?.addEventListener) { - return; - } + if (!resizingParent?.addEventListener) return; + const handler = () => { - setTargetRect(targetRef?.current?.getBoundingClientRect()); + setTargetRect(resolvedTarget.getBoundingClientRect()); }; const ro = new ResizeObserver(handler); ro.observe(resizingParent); return () => ro.unobserve(resizingParent); - }, [targetRef, setTargetRect]); + }, [resolvedTarget, setTargetRect]); }; /** * Memoizes the list of scrolling parent elements for a target element. - * This avoids expensive DOM traversals and getComputedStyle calls on every render. * Returns an empty array if the target element is not available. */ export const useScrollingParents = ( - targetRef: React.RefObject + targetRef: RefObject ): HTMLElement[] => { + const resolvedTarget = useResolvedRefTarget(targetRef); + return useMemo(() => { - if (!targetRef.current) { - return []; - } - return findAllAdditionalScrollingParents(targetRef.current); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [targetRef.current]); + if (!resolvedTarget) return []; + return findAllAdditionalScrollingParents( + getTargetAsElement(resolvedTarget)! + ); + }, [resolvedTarget]); }; diff --git a/packages/gamut/src/PopoverContainer/types.ts b/packages/gamut/src/PopoverContainer/types.ts index 8f958c921d0..94a6744b620 100644 --- a/packages/gamut/src/PopoverContainer/types.ts +++ b/packages/gamut/src/PopoverContainer/types.ts @@ -83,8 +83,9 @@ export interface PopoverContainerProps onRequestClose?: () => void; /** * The target element around which the popover will be positioned. + * Only ref objects (e.g. from useRef) are supported at runtime; RefCallback is not. */ - targetRef: RefObject; + targetRef: RefObject; /** * If true, it will allow outside page interaction. Popover container will still close when clicking outside of the popover or hitting the escape key. */ diff --git a/packages/gamut/src/Tag/index.tsx b/packages/gamut/src/Tag/index.tsx index 686478ff325..76b96f95145 100644 --- a/packages/gamut/src/Tag/index.tsx +++ b/packages/gamut/src/Tag/index.tsx @@ -89,7 +89,13 @@ export const Tag: React.FC = ({ {isSelection && ( ) => { +export const createFocusOnClick = (ref: RefObject) => { return ({ isTipHidden }: { isTipHidden: boolean }) => { if (!isTipHidden) ref.current?.focus(); }; @@ -419,24 +419,18 @@ export const openInfoTipsWithKeyboard = async ({ }) => { const buttons = view.getAllByLabelText('Show information'); - await act(async () => { - buttons[0].focus(); - await userEvent.keyboard('{Enter}'); - - for (let i = 1; i < count; i += 1) { - // eslint-disable-next-line no-await-in-loop - await userEvent.tab(); - // eslint-disable-next-line no-await-in-loop + const openNext = async (i: number): Promise => { + if (i >= count) return; + await act(async () => { + buttons[i].focus(); await userEvent.keyboard('{Enter}'); - } - }); - - // Wait for all tips to finish opening - await waitFor(() => { - buttons.forEach((button) => { - expect(button).toHaveAttribute('aria-expanded', 'true'); }); - }); + await waitFor(() => { + expect(buttons[i]).toHaveAttribute('aria-expanded', 'true'); + }); + await openNext(i + 1); + }; + await openNext(0); }; export const expectTipsVisible = (tips: { text: string }[]) => { diff --git a/packages/gamut/src/Tip/shared/FloatingTip.tsx b/packages/gamut/src/Tip/shared/FloatingTip.tsx index c3013661549..ce1a8541d49 100644 --- a/packages/gamut/src/Tip/shared/FloatingTip.tsx +++ b/packages/gamut/src/Tip/shared/FloatingTip.tsx @@ -43,8 +43,8 @@ export const FloatingTip: React.FC = ({ const [isFocused, setIsFocused] = useState(false); // Use refs to store timeouts to prevent race conditions - const hoverDelayRef = useRef(); - const focusDelayRef = useRef(); + const hoverDelayRef = useRef(undefined); + const focusDelayRef = useRef(undefined); const commonPopoverProps = getPopoverAlignmentAndPattern({ alignment, type }); const dims = getAlignmentStyles({ avatar, alignment, type }); @@ -142,13 +142,13 @@ export const FloatingTip: React.FC = ({ display="inline-flex" height={inheritDims ? 'inherit' : undefined} position="relative" - ref={wrapperRef} + ref={wrapperRef as React.Ref} width={inheritDims ? 'inherit' : undefined} onMouseLeave={toolOnlyEventFunc} > } width={inheritDims ? 'inherit' : undefined} onBlur={toolOnlyEventFunc} onFocus={toolOnlyEventFunc} diff --git a/packages/gamut/src/Tip/shared/InlineTip.tsx b/packages/gamut/src/Tip/shared/InlineTip.tsx index 871cfaac1ac..230ef200950 100644 --- a/packages/gamut/src/Tip/shared/InlineTip.tsx +++ b/packages/gamut/src/Tip/shared/InlineTip.tsx @@ -44,7 +44,7 @@ export const InlineTip: React.FC = ({ const target = ( } width={inheritDims ? 'inherit' : undefined} onKeyDown={escapeKeyPressHandler} > @@ -64,7 +64,7 @@ export const InlineTip: React.FC = ({ color="currentColor" horizNarrow={narrow && isHorizontalCenter} id={id} - ref={contentRef} + ref={contentRef as React.Ref} role={type === 'tool' ? 'tooltip' : undefined} tabIndex={type === 'info' ? -1 : undefined} width={narrow && !isHorizontalCenter ? narrowWidth : 'max-content'} diff --git a/packages/gamut/src/Tip/shared/types.tsx b/packages/gamut/src/Tip/shared/types.tsx index 164ed1aaf37..32299dd678e 100644 --- a/packages/gamut/src/Tip/shared/types.tsx +++ b/packages/gamut/src/Tip/shared/types.tsx @@ -78,10 +78,8 @@ export type TipPlacementComponentProps = Omit< escapeKeyPressHandler?: (event: React.KeyboardEvent) => void; id?: string; isTipHidden?: boolean; - contentRef?: - | React.RefObject - | ((node: HTMLDivElement | null) => void); + contentRef?: React.Ref; type: 'info' | 'tool' | 'preview'; - wrapperRef?: React.RefObject; + wrapperRef?: React.Ref; zIndex?: number; } & React.PropsWithChildren; diff --git a/packages/gamut/src/Tip/shared/utils.tsx b/packages/gamut/src/Tip/shared/utils.tsx index 2ca69443b09..0018f3ed33b 100755 --- a/packages/gamut/src/Tip/shared/utils.tsx +++ b/packages/gamut/src/Tip/shared/utils.tsx @@ -101,7 +101,7 @@ export const isFloatingElementOpen = (element: Element): boolean => { const toggleButton = element.querySelector( 'button[aria-expanded], [role="button"][aria-expanded]' ); - if (toggleButton && toggleButton.getAttribute('aria-expanded') === 'false') { + if (toggleButton?.getAttribute('aria-expanded') === 'false') { return false; } diff --git a/packages/gamut/src/utils/__tests__/nullish.test.ts b/packages/gamut/src/utils/__tests__/nullish.test.ts new file mode 100644 index 00000000000..c44212e77eb --- /dev/null +++ b/packages/gamut/src/utils/__tests__/nullish.test.ts @@ -0,0 +1,38 @@ +import { isDefined, isNullish } from '../nullish'; + +describe('isNullish', () => { + it('is true for null and undefined', () => { + expect(isNullish(null)).toBe(true); + expect(isNullish(undefined)).toBe(true); + }); + + it('is false for other values', () => { + expect(isNullish(0)).toBe(false); + expect(isNullish('')).toBe(false); + expect(isNullish(false)).toBe(false); + expect(isNullish({})).toBe(false); + }); +}); + +describe('isDefined', () => { + it('is false for null and undefined', () => { + expect(isDefined(null)).toBe(false); + expect(isDefined(undefined)).toBe(false); + }); + + it('is true for other values', () => { + expect(isDefined(0)).toBe(true); + expect(isDefined('')).toBe(true); + expect(isDefined(false)).toBe(true); + expect(isDefined({})).toBe(true); + }); +}); + +describe('isNullish and isDefined', () => { + it('are opposites for typical inputs', () => { + const values: unknown[] = [null, undefined, 0, '', NaN, Symbol('x')]; + for (const v of values) { + expect(isNullish(v)).toBe(!isDefined(v)); + } + }); +}); diff --git a/packages/gamut/src/utils/nullish.ts b/packages/gamut/src/utils/nullish.ts new file mode 100644 index 00000000000..b0a20d6ca4b --- /dev/null +++ b/packages/gamut/src/utils/nullish.ts @@ -0,0 +1,13 @@ +/** + * True when `value` is `null` or `undefined`. + * Use instead of `value == null` when `eqeqeq` is enforced. + */ +export const isNullish = (value: unknown): value is null | undefined => + value === null || value === undefined; + +/** + * True when `value` is neither `null` nor `undefined`. + * Use instead of `value != null` when `eqeqeq` is enforced. + */ +export const isDefined = (value: T | null | undefined): value is T => + value !== undefined && value !== null; diff --git a/packages/gamut/src/utils/react.ts b/packages/gamut/src/utils/react.ts index c0b673bb24d..904f855cabd 100644 --- a/packages/gamut/src/utils/react.ts +++ b/packages/gamut/src/utils/react.ts @@ -1,5 +1,7 @@ import { Children, isValidElement } from 'react'; +import { isNullish } from './nullish'; + /** * Recursively extracts plain text content from React children. * @@ -31,11 +33,15 @@ export const extractTextContent = (children: React.ReactNode): string => { if (typeof child === 'string' || typeof child === 'number') { return String(child); } - if (typeof child === 'boolean' || child == null) { + if (typeof child === 'boolean' || isNullish(child)) { return ''; } if (isValidElement(child)) { - const textContent = child.props.children ?? child.props.text ?? ''; + const props = child.props as { + children?: React.ReactNode; + text?: string; + }; + const textContent = props.children ?? props.text ?? ''; return extractTextContent(textContent); } return ''; diff --git a/packages/styleguide/src/lib/Atoms/FormInputs/SelectDropdown/SelectDropdown.mdx b/packages/styleguide/src/lib/Atoms/FormInputs/SelectDropdown/SelectDropdown.mdx index 57d4b1b123d..c1515cf5b0c 100644 --- a/packages/styleguide/src/lib/Atoms/FormInputs/SelectDropdown/SelectDropdown.mdx +++ b/packages/styleguide/src/lib/Atoms/FormInputs/SelectDropdown/SelectDropdown.mdx @@ -2,6 +2,8 @@ import { Canvas, Controls, Meta } from '@storybook/addon-docs/blocks'; import { Callout, ComponentHeader, LinkTo } from '~styleguide/blocks'; +import { ControlledModeTable } from './controlledModeTable'; +import { CreatablePropsTable } from './creatablePropsTable'; import * as SelectDropdownStories from './SelectDropdown.stories'; export const parameters = { @@ -15,7 +17,7 @@ export const parameters = { source: { repo: 'gamut', githubLink: - 'https://github.com/Codecademy/gamut/blob/main/packages/gamut/src/Form/inputs/SelectDropdown.tsx', + 'https://github.com/Codecademy/gamut/blob/main/packages/gamut/src/Form/SelectDropdown/SelectDropdown.tsx', }, }; @@ -24,84 +26,32 @@ export const parameters = { ## Usage -Use SelectDropdown to pick from exclusive options within a styled dropdown. +Use `SelectDropdown` when designs call for a styled dropdown menu, searchable options, multi-select tags, creatable options, or rich option content (icons, subtitles, grouping). It carries a larger JavaScript dependency than the standard `Select` component (react-select), so prefer `Select` for simple single-select fields where that extra weight isn't justified by the design. -`SelectDropdown` has a larger JavaScript dependency size than our standard `Select` component so should not be used unless specified in designs. These dropdowns are optionally searchable through the `isSearchable` prop. +When used inside a form, provide both a `name` prop and an `aria-label` that matches the `htmlFor` of the associated `FormGroupLabel` or `