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/.github/actions/prerelease-publish/action.yml b/.github/actions/prerelease-publish/action.yml new file mode 100644 index 00000000000..ac33186c08f --- /dev/null +++ b/.github/actions/prerelease-publish/action.yml @@ -0,0 +1,127 @@ +name: PR prerelease npm publish +description: Checkout, setup, build publish targets, gamut-release, and sticky PR comment + +inputs: + node-auth-token: + description: NPM token value (pass secrets.NODE_AUTH_TOKEN) + required: true + gh-token: + description: GitHub token for release tooling (pass secrets.ACTIONS_GITHUB_TOKEN) + required: true + ignore-commit-message: + description: Commit message prefix to skip (chore release commits) + required: true + manifest-path: + description: JSON manifest path written by gamut-release + required: true + comment-path: + description: Markdown file path for the PR comment body + required: true + comment-title: + description: Title line after the emoji (e.g. Published Alpha Packages) + required: true + sticky-header: + description: Sticky comment header key + required: true + publish-run: + description: Bash to set PREID and run gamut-release:pre-release + required: true + +runs: + using: composite + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + filter: tree:0 + ref: ${{ github.event.pull_request.head.ref }} + + - name: Setup and Build + id: setup + uses: ./.github/actions/yarn + + - name: Set git user + uses: ./.github/actions/set-git-user + + - name: Set npm token + uses: ./.github/actions/set-npm-token + with: + token-secret: ${{ inputs.node-auth-token }} + + - name: Ensure workflow is associated with a pull request + uses: ./.github/actions/validate-pr-context + + - name: Skip build from automated commit + uses: ./.github/actions/skip-automated-commits + with: + ignore-commit-message: ${{ inputs.ignore-commit-message }} + + - run: npx nx run-many --target=publish-build --parallel=3 + shell: bash + + - name: Publish prerelease packages + id: publish-prerelease + shell: bash + env: + GH_TOKEN: ${{ inputs.gh-token }} + NODE_AUTH_TOKEN: ${{ inputs.node-auth-token }} + run: | + ${{ inputs.publish-run }} + + - name: Build prerelease packages comment + id: build-comment + continue-on-error: true + uses: actions/github-script@v7 + env: + MANIFEST_PATH: ${{ inputs.manifest-path }} + OUTPUT_PATH: ${{ inputs.comment-path }} + COMMENT_TITLE: ${{ inputs.comment-title }} + with: + script: | + const fs = require('fs'); + const core = require('@actions/core'); + const manifestPath = process.env.MANIFEST_PATH; + const outputPath = process.env.OUTPUT_PATH; + const commentTitle = process.env.COMMENT_TITLE; + if (!fs.existsSync(manifestPath)) { + fs.writeFileSync(outputPath, ''); + core.setOutput('has_comment', 'false'); + return; + } + + const entries = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); + if (!Array.isArray(entries) || entries.length === 0) { + fs.writeFileSync(outputPath, ''); + core.setOutput('has_comment', 'false'); + return; + } + + const tableLines = [ + '| Package | Version | npm | Diff |', + '| --- | --- | --- | --- |', + ...entries.map((entry) => { + const pkgUrl = `https://www.npmjs.com/package/${entry.name}`; + const versionUrl = `${pkgUrl}/v/${entry.version}`; + const baseVersion = entry.previousVersion || null; + const diffUrl = baseVersion + ? `https://npmdiff.dev/${encodeURIComponent( + entry.name + )}/${baseVersion}/${entry.version}/` + : null; + const npmLink = `[npm](${versionUrl})`; + const diffLink = diffUrl ? `[diff](${diffUrl})` : 'N/A'; + + return `| \`${entry.name}\` | \`${entry.version}\` | ${npmLink} | ${diffLink} |`; + }), + ]; + const comment = `📬 ${commentTitle}\n\n${tableLines.join('\n')}\n`; + fs.writeFileSync(outputPath, comment); + core.setOutput('has_comment', 'true'); + + - name: Comment with published packages + uses: ./.github/actions/sticky-comment + if: ${{ !cancelled() && steps.build-comment.outputs.has_comment == 'true' }} + with: + github-token: ${{ inputs.gh-token }} + header: ${{ inputs.sticky-header }} + message-path: ${{ inputs.comment-path }} diff --git a/.github/workflows/publish-alpha.yml b/.github/workflows/publish-alpha.yml index b1d926c845c..e4c5830d8a6 100644 --- a/.github/workflows/publish-alpha.yml +++ b/.github/workflows/publish-alpha.yml @@ -17,94 +17,28 @@ permissions: issues: write actions: write +concurrency: + group: npm-prerelease-${{ github.event.pull_request.number }} + cancel-in-progress: false + jobs: publish-alpha: + if: ${{ !contains(github.event.pull_request.labels.*.name, 'beta') }} runs-on: ubuntu-22.04 + timeout-minutes: 30 steps: - uses: actions/checkout@v4 - with: - fetch-depth: 0 - ref: ${{ github.event.pull_request.head.ref }} - - - name: Setup and Build - id: setup - uses: ./.github/actions/yarn - - - name: Set git user - uses: ./.github/actions/set-git-user - - name: Set npm token - uses: ./.github/actions/set-npm-token - with: - token-secret: ${{ secrets.NODE_AUTH_TOKEN }} - - - name: Ensure workflow is associated with a pull request - uses: ./.github/actions/validate-pr-context - - - name: Skip build from automated commit - uses: ./.github/actions/skip-automated-commits + - uses: ./.github/actions/prerelease-publish with: + node-auth-token: ${{ secrets.NODE_AUTH_TOKEN }} + gh-token: ${{ secrets.ACTIONS_GITHUB_TOKEN }} ignore-commit-message: ${{ env.IGNORE_COMMIT_MESSAGE }} - - - run: npx nx run-many --target=publish-build --parallel=3 - - - name: Publish alpha packages - id: publish-alpha - run: | - SHORT_SHA=${GITHUB_SHA:0:6} - PREID="alpha.${SHORT_SHA}" - npx nx run gamut-release:alpha --preid="${PREID}" --manifest - env: - GH_TOKEN: ${{ secrets.ACTIONS_GITHUB_TOKEN }} - NODE_AUTH_TOKEN: ${{ secrets.NODE_AUTH_TOKEN }} - - - name: List alpha packages versions - id: published - continue-on-error: true - uses: actions/github-script@v7 - with: - script: | - const fs = require('fs'); - const manifestPath = 'alpha-publish-manifest.json'; - const outputPath = 'alpha-publish-comment.md'; - if (!fs.existsSync(manifestPath)) { - fs.writeFileSync(outputPath, ''); - return; - } - - const entries = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); - if (!Array.isArray(entries) || entries.length === 0) { - fs.writeFileSync(outputPath, ''); - return; - } - - const tableLines = [ - '| Package | Version | npm | Diff |', - '| --- | --- | --- | --- |', - ...entries.map((entry) => { - const pkgUrl = `https://www.npmjs.com/package/${entry.name}`; - const versionUrl = `${pkgUrl}/v/${entry.version}`; - const baseVersion = entry.previousVersion || null; - const diffUrl = baseVersion - ? `https://npmdiff.dev/${encodeURIComponent( - entry.name - )}/${baseVersion}/${entry.version}/` - : null; - const npmLink = `[npm](${versionUrl})`; - const diffLink = diffUrl ? `[diff](${diffUrl})` : 'N/A'; - - return `| \`${entry.name}\` | \`${entry.version}\` | ${npmLink} | ${diffLink} |`; - }), - ]; - const comment = `📬 Published Alpha Packages:\n\n${tableLines.join( - '\n' - )}\n`; - fs.writeFileSync(outputPath, comment); - - - name: Comment with published alpha packages - uses: ./.github/actions/sticky-comment - if: ${{ !cancelled() && hashFiles('alpha-publish-comment.md') != '' }} - with: - github-token: ${{ secrets.ACTIONS_GITHUB_TOKEN }} - header: Alpha Packages - message-path: alpha-publish-comment.md + manifest-path: alpha-publish-manifest.json + comment-path: alpha-publish-comment.md + comment-title: Published Alpha Packages + sticky-header: Alpha Packages + publish-run: | + SHORT_SHA=${GITHUB_SHA:0:6} + PREID="alpha.${SHORT_SHA}" + npx nx run gamut-release:pre-release --preid="${PREID}" --manifest diff --git a/.github/workflows/publish-beta.yml b/.github/workflows/publish-beta.yml new file mode 100644 index 00000000000..2578cd6249f --- /dev/null +++ b/.github/workflows/publish-beta.yml @@ -0,0 +1,43 @@ +name: Publish Beta + +on: + pull_request: + types: [labeled] + +env: + NODE_VERSION: '22.13.1' + NODE_OPTIONS: '--max_old_space_size=8196' + NX_CLOUD: false + IGNORE_COMMIT_MESSAGE: 'chore(release): publish' + +permissions: + id-token: write + contents: read + pull-requests: write + issues: write + actions: write + +concurrency: + group: npm-prerelease-${{ github.event.pull_request.number }} + cancel-in-progress: false + +jobs: + publish-beta: + if: github.event.label.name == 'beta' && github.event.pull_request.head.ref == 'cass-gmt-1607-publish' + runs-on: ubuntu-22.04 + timeout-minutes: 30 + steps: + - uses: actions/checkout@v4 + + - uses: ./.github/actions/prerelease-publish + with: + node-auth-token: ${{ secrets.NODE_AUTH_TOKEN }} + gh-token: ${{ secrets.ACTIONS_GITHUB_TOKEN }} + ignore-commit-message: ${{ env.IGNORE_COMMIT_MESSAGE }} + manifest-path: beta-publish-manifest.json + comment-path: beta-publish-comment.md + comment-title: 'Published Beta Packages (install with `@beta`):' + sticky-header: Beta Packages + publish-run: | + PREID="beta.pr${{ github.event.pull_request.number }}.${{ github.run_id }}" + npx nx run gamut-release:pre-release --preid="${PREID}" --publish-tag=beta --manifest=beta-publish-manifest.json 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/README.md b/README.md index 84f0e396661..0d3957b39ad 100644 --- a/README.md +++ b/README.md @@ -108,17 +108,21 @@ Add new Button variant and fix spacing issues - It will create git tags and GitHub releases 1. You can find the new version number on npmjs.com/package/, or find it in that package's `package.json` on the `main` branch -### Publishing an alpha version of a module +### Publishing an pre-release version of a module -Every PR that changes files in a package publishes alpha releases that you can use to test your changes across applications. +You can consume prerelease packages from npm before your PR merges. Two flows are available; **alpha** and **beta** are mutually exclusive on a PR. + +**Alpha (default):** On each push to the PR branch, CI publishes alpha releases (per-commit prerelease versions on npm). This does **not** run while the PR has the **`beta`** label. + +**Beta:** Add the **`beta`** label to the PR (create the label in the repo if it does not exist). That triggers a one-time publish to npm under the **`beta`** dist-tag (install with e.g. `yarn add @codecademy/gamut@beta`). Pushing new commits does **not** republish beta automatically; remove the **`beta`** label and add it again after your changes to trigger another publish. > NOTE: in case an alpha build is not published upon opening of the PR or Draft PR, re-run the `build-test` check and that will re-run the alpha build publishing flows 1. Create a PR or Draft PR. - - This will kickoff a Github Action workflow which will publish an alpha build. (This will appear in Github as the "Deploy") -1. After the alpha build is published, the `codecademydev` bot should comment on your PR with the names of the published alpha packages.
+ - Without the **`beta`** label, each push kicks off the workflow that publishes an alpha build. (This will appear in Github as the "Deploy") +1. After packages are published, the `codecademydev` bot should comment on your PR with the names of the published packages (separate comments for alpha vs beta).
-1. Install this version of the package in your application you wish to test your changes on. +1. Install that version in the application where you want to test your changes (alpha by exact version from the table, or beta via `@beta` when using the beta flow). ### Working with pre-published changes 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..6b97c4d49f1 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,8 +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/gamut": "workspace:*", + "@codecademy/eslint-config": "8.2.0", "@codecademy/prettier-config": "^0.2.0", "@codecademy/tsconfig": "^0.3.0", "@commander-js/extra-typings": "^14.0.0", @@ -43,27 +42,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 +81,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 +112,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-kit/CHANGELOG.md b/packages/gamut-kit/CHANGELOG.md index 8d20160c46f..3bdbfe5fef4 100644 --- a/packages/gamut-kit/CHANGELOG.md +++ b/packages/gamut-kit/CHANGELOG.md @@ -17,7 +17,7 @@ # 3.0.0 (2026-06-15) -### ⚠️ Breaking Changes +### ⚠️ Breaking Changes - Remove deprecated accordion components ([#3366](https://github.com/Codecademy/gamut/pull/3366)) 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..12d27097f82 100644 --- a/packages/gamut-tests/package.json +++ b/packages/gamut-tests/package.json @@ -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/CHANGELOG.md b/packages/gamut/CHANGELOG.md index 39a268984bd..ac1bf32184f 100644 --- a/packages/gamut/CHANGELOG.md +++ b/packages/gamut/CHANGELOG.md @@ -20,7 +20,7 @@ # 72.0.0 (2026-06-15) -### ⚠️ Breaking Changes +### ⚠️ Breaking Changes - Remove deprecated accordion components ([#3366](https://github.com/Codecademy/gamut/pull/3366)) diff --git a/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md b/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md index ea61d364efa..758001eb5fd 100644 --- a/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md +++ b/packages/gamut/agent-tools/skills/gamut-forms/SKILL.md @@ -26,6 +26,12 @@ For typical product forms, prefer `GridForm` (declarative `fields`, `LayoutGrid` --- +## SelectDropdown + +For `SelectDropdown` — single vs multi value, controlled vs uncontrolled patterns, creatable options, and 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; SelectDropdown-specific state contracts live in that skill. + +--- + ## `FormGroup` (baseline) [`FormGroup.tsx`](https://github.com/Codecademy/gamut/blob/main/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..a44fcabafd3 --- /dev/null +++ b/packages/gamut/agent-tools/skills/gamut-select-dropdown/SKILL.md @@ -0,0 +1,183 @@ +--- +name: gamut-select-dropdown +description: Use when implementing or auditing SelectDropdown — single/multi modes, controlled vs uncontrolled value, creatable options, FormGroup wiring, and react-select action meta. Pair with gamut-forms for FormGroup/validation patterns. +--- + +# Gamut SelectDropdown + +Styled dropdown built on react-select. Supports single and multi-select, searchable menus, creatable options, icons, groups, and abbreviations. + +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. + +--- + +## 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. Handle `onInputChange`: validate on `input-change`, re-validate from last typed value on `input-blur` so FormGroup error persists. + +--- + +## FormGroup wiring + +- `FormGroup` `htmlFor` must match control `id` / `name`. +- Pass `name` on SelectDropdown (required for forms). +- Pass `aria-label` (required for forms); it must match the FormGroupLabel `htmlFor` / `name`. +- 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 1819205b9db..7ad7bccf8e9 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 97f17ca74c0..d007b803a43 100644 --- a/packages/gamut/src/ConnectedForm/ConnectedFormGroup.tsx +++ b/packages/gamut/src/ConnectedForm/ConnectedFormGroup.tsx @@ -60,6 +60,7 @@ export function ConnectedFormGroup({ isSoloField, infotip, }: ConnectedFormGroupProps) { + const fieldId = typeof id === 'string' && id !== '' ? id : name; const { error, isFirstError, isDisabled, setError, validation } = useField({ name, disabled, @@ -78,7 +79,7 @@ export function ConnectedFormGroup({ 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 95560991c7f..ef0a3fb9c31 100644 --- a/packages/gamut/src/ConnectedForm/utils.tsx +++ b/packages/gamut/src/ConnectedForm/utils.tsx @@ -177,9 +177,7 @@ export const useField = ({ name, disabled, loading }: useFieldProps) => { }); const validation = - (validationRules && - validationRules[name as keyof typeof validationRules]) ?? - undefined; + validationRules?.[name as keyof typeof validationRules] ?? undefined; const ref = register(name, validation); @@ -333,7 +331,6 @@ type InputTypes = | 'search' | 'month' | 'tel' - | 'time' | 'url' | 'week' > 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..8dd980e6c9c 100644 --- a/packages/gamut/src/Form/SelectDropdown/SelectDropdown.tsx +++ b/packages/gamut/src/Form/SelectDropdown/SelectDropdown.tsx @@ -9,7 +9,7 @@ import { useState, } from 'react'; import * as React from 'react'; -import { Options as OptionsType, StylesConfig } from 'react-select'; +import { ActionMeta, Options as OptionsType, StylesConfig } from 'react-select'; import { parseOptions, SelectOptionBase } from '../utils'; import { @@ -36,6 +36,7 @@ import { } from './types'; import { filterValueFromOptions, + getCreatedOptionValue, isMultipleSelectProps, isOptionsGrouped, isSingleSelectProps, @@ -109,22 +110,30 @@ 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}`; @@ -180,8 +189,11 @@ export const SelectDropdown: React.FC = ({ ) ); - // If the caller changes the initial value, let's update our value to match. + // 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, @@ -189,38 +201,56 @@ export const SelectDropdown: React.FC = ({ ); 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]); + }, [options, value, multiple]); const changeHandler = useCallback( - (optionEvent: OptionStrict | OptionsType) => { + ( + optionEvent: OptionStrict | OptionsType, + actionMeta: ActionMeta + ) => { 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 + 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: onChangeAction, + option: isMultipleSelectProps(onChangeProps) + ? undefined + : (optionEvent as OptionStrict), + }; if (isSingleSelectProps(onChangeProps)) { const singleOptionEvent = optionEvent as OptionStrict; - onChangeProps.onChange?.(singleOptionEvent, { - action: onChangeAction, - option: singleOptionEvent, - }); + onChangeProps.onChange?.(singleOptionEvent, forwardedMeta); } 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) - }); + onChangeProps.onChange?.( + optionEvent as OptionsType, + forwardedMeta + ); } }, - [onChange, multiple] + [onChange, multiple, onCreateOption] ); const keyPressHandler = (e: KeyboardEvent) => { @@ -242,6 +272,13 @@ export const SelectDropdown: React.FC = ({ } }; + const noOptionsMessage = + validationMessage === undefined + ? undefined // fall back to react-select default ("No options") + : typeof validationMessage === 'function' + ? (validationMessage as (obj: { inputValue: string }) => React.ReactNode) + : () => validationMessage; + const theme = useTheme(); const memoizedStyles = useMemo((): StylesConfig => { return getMemoizedStyles(theme, zIndex); @@ -265,18 +302,24 @@ export const SelectDropdown: React.FC = ({ }} dropdownWidth={dropdownWidth} error={Boolean(error)} + formatCreateLabel={formatCreateLabel} formatGroupLabel={formatGroupLabel} formatOptionLabel={formatOptionLabel} id={id || rest.htmlFor || rawInputId} inputId={inputId} inputProps={{ ...inputProps }} inputWidth={inputWidth} + isCreatable={isCreatable} isDisabled={disabled} isMulti={multiple} - isOptionDisabled={(option) => option.disabled} + isOptionDisabled={(option: OptionStrict & { disabled?: boolean }) => + option.disabled ?? false + } isSearchable={isSearchable} + isValidNewOption={isValidNewOption} menuAlignment={menuAlignment} name={name} + noOptionsMessage={noOptionsMessage} options={selectOptions} placeholder={placeholder} selectRef={selectInputRef} @@ -285,7 +328,12 @@ export const SelectDropdown: React.FC = ({ styles={memoizedStyles} value={multiple ? multiValues : parsedValue} onChange={changeHandler} - onKeyDown={multiple ? (e) => keyPressHandler(e) : undefined} + onInputChange={onInputChange} + onKeyDown={ + multiple + ? (e: React.KeyboardEvent) => keyPressHandler(e) + : 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..a1a244babde 100644 --- a/packages/gamut/src/Form/SelectDropdown/__tests__/utils.test.tsx +++ b/packages/gamut/src/Form/SelectDropdown/__tests__/utils.test.tsx @@ -2,12 +2,71 @@ import { SelectOptionBase } from '../../utils'; import { ExtendedOption, SelectDropdownGroup } from '../types'; import { filterValueFromOptions, + getCreatedOptionValue, isOptionGroup, isOptionsGrouped, removeValueFromSelectedOptions, } from '../utils'; 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/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..079915cb398 100644 --- a/packages/gamut/src/Form/SelectDropdown/elements/controls.tsx +++ b/packages/gamut/src/Form/SelectDropdown/elements/controls.tsx @@ -37,11 +37,10 @@ export const onFocus: AriaOnFocus = ({ * 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 ( @@ -90,7 +89,7 @@ export const RemoveAllButton = (props: SizedIndicatorProps) => { selectInputRef?.current && (e.key === 'ArrowRight' || e.key === 'ArrowLeft' || e.key === 'ArrowDown') ) { - selectInputRef?.current.focus(); + selectInputRef.current.focus(); } }; @@ -102,7 +101,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/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/styles.ts b/packages/gamut/src/Form/SelectDropdown/styles.ts index ba9178c113c..a7fc50142fa 100644 --- a/packages/gamut/src/Form/SelectDropdown/styles.ts +++ b/packages/gamut/src/Form/SelectDropdown/styles.ts @@ -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,36 @@ 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: isNew + ? state.isDisabled + ? theme.colors['text-disabled'] + : theme.colors.primary + : state.isDisabled + ? theme.colors['text-disabled'] + : theme.colors.text, + cursor: state.isDisabled ? 'not-allowed' : 'pointer', + display: 'flex', + padding: isSmall ? '3px 14px' : '11px 14px', + ...(isNew && { + // Gradient creates the 1px divider line centred in the 16px spacer above the option text + backgroundImage: `linear-gradient(${theme.colors['text-disabled']} 1px, transparent 1px)`, + backgroundPosition: '0 8px', + backgroundRepeat: 'no-repeat', + backgroundSize: '100% 1px', + paddingTop: isSmall ? '19px' : '27px', + }), + }; + }, placeholder: (provided) => ({ ...provided, ...placeholderColor({ theme }), diff --git a/packages/gamut/src/Form/SelectDropdown/types/component-props.ts b/packages/gamut/src/Form/SelectDropdown/types/component-props.ts index d5c89453083..37fd704ab52 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,19 @@ 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 { /** Optional ref to the underlying react-select component */ selectRef?: Ref; + /** When true, renders CreatableSelect instead of ReactSelect */ + isCreatable?: boolean; + /** Customises the "Add" row label */ + formatCreateLabel?: (inputValue: string) => React.ReactNode; + /** Controls visibility of the "Add" row */ + isValidNewOption?: ( + inputValue: string, + value: OptionsType, + options: OptionsType + ) => boolean; } diff --git a/packages/gamut/src/Form/SelectDropdown/types/internal.ts b/packages/gamut/src/Form/SelectDropdown/types/internal.ts index 8a151e2102c..fabd1307227 100644 --- a/packages/gamut/src/Form/SelectDropdown/types/internal.ts +++ b/packages/gamut/src/Form/SelectDropdown/types/internal.ts @@ -14,12 +14,10 @@ export type InternalSelectProps = { }; /** - * Ref type for programmatic focus management. + * Ref type for programmatic focus management (internal refs from useRef). * Used for managing focus on select input and remove all button. */ -export type ProgramaticFocusRef = - | React.MutableRefObject - | React.MutableRefObject; +export type ProgramaticFocusRef = React.RefObject; /** * Context value for SelectDropdown internal state management. diff --git a/packages/gamut/src/Form/SelectDropdown/types/styles.ts b/packages/gamut/src/Form/SelectDropdown/types/styles.ts index 3be7acbd824..23e7f2334ec 100644 --- a/packages/gamut/src/Form/SelectDropdown/types/styles.ts +++ b/packages/gamut/src/Form/SelectDropdown/types/styles.ts @@ -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/SelectDropdown/utils.tsx b/packages/gamut/src/Form/SelectDropdown/utils.tsx index bcf0fd7ac87..6363a3286cc 100644 --- a/packages/gamut/src/Form/SelectDropdown/utils.tsx +++ b/packages/gamut/src/Form/SelectDropdown/utils.tsx @@ -1,8 +1,12 @@ +import { ActionMeta, Options as OptionsType } from 'react-select'; + +import { isDefined } from '../../utils/nullish'; import { SelectOptionBase } from '../utils'; import { BaseOnChangeProps, ExtendedOption, MultiSelectDropdownProps, + OptionStrict, SelectDropdownGroup, SelectDropdownOptions, SelectDropdownProps, @@ -17,9 +21,36 @@ 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 && + isDefined(obj) && typeof obj === 'object' && + obj !== null && 'options' in obj && obj.options !== undefined; 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/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 0429e639192..87010792494 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 @@ -133,15 +145,13 @@ export const MenuItem = forwardRef< ); if (listItemType === 'link' && !disabled) { - const linkRef = ref as MutableRefObject; - return ( (ref)} target={target} > {content} @@ -152,7 +162,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); @@ -162,7 +171,7 @@ export const MenuItem = forwardRef< (ref)} onClick={handleClick} > {content} @@ -172,11 +181,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 29626f3dbe5..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 3b09f0209ad..815fa9cda38 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', }, }; @@ -30,44 +32,7 @@ Use SelectDropdown to pick from exclusive options within a styled dropdown. If you are using this in a standard form, `SelectDropdown` must be provided an `aria-label` and a `name`. The `aria-label` must match the `htmlFor` of the FormGroupLabel or `