From 486ff8dbcab71e7a0440e72013c1d83065d205c1 Mon Sep 17 00:00:00 2001 From: Dimitri POSTOLOV Date: Tue, 23 May 2023 02:49:52 +0200 Subject: [PATCH 1/5] replace `@reach/combobox` by `cmdk` --- .../package.json | 4 +- .../graphiql-plugin-explorer/package.json | 4 +- packages/graphiql-react/package.json | 10 +- .../src/explorer/components/doc-explorer.css | 4 +- .../src/explorer/components/search.css | 16 +- .../src/explorer/components/search.tsx | 201 +++--- packages/graphiql-react/src/toolbar/menu.tsx | 1 + packages/graphiql/cypress/e2e/docs.cy.ts | 26 +- packages/graphiql/package.json | 8 +- packages/graphiql/resources/index.html.ejs | 12 +- packages/graphiql/resources/renderExample.js | 5 +- yarn.lock | 681 ++++++++++++------ 12 files changed, 590 insertions(+), 382 deletions(-) diff --git a/packages/graphiql-plugin-code-exporter/package.json b/packages/graphiql-plugin-code-exporter/package.json index a7d9e2f4973..1a5cd910cab 100644 --- a/packages/graphiql-plugin-code-exporter/package.json +++ b/packages/graphiql-plugin-code-exporter/package.json @@ -33,8 +33,8 @@ }, "peerDependencies": { "graphql": "^15.5.0 || ^16.0.0", - "react": "^16.8.0 || ^17.0.0 || ^18.0.0", - "react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0" + "react": "^18.2.0", + "react-dom": "^18.2.0" }, "devDependencies": { "@graphiql/react": "^0.17.4", diff --git a/packages/graphiql-plugin-explorer/package.json b/packages/graphiql-plugin-explorer/package.json index 93bc9203369..7ee4bfb16b3 100644 --- a/packages/graphiql-plugin-explorer/package.json +++ b/packages/graphiql-plugin-explorer/package.json @@ -33,8 +33,8 @@ }, "peerDependencies": { "graphql": "^15.5.0 || ^16.0.0", - "react": "^16.8.0 || ^17.0.0 || ^18.0.0", - "react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0" + "react": "^18.2.0", + "react-dom": "^18.2.0" }, "devDependencies": { "@vitejs/plugin-react": "^1.3.0", diff --git a/packages/graphiql-react/package.json b/packages/graphiql-react/package.json index 2a638211260..45319b0eada 100644 --- a/packages/graphiql-react/package.json +++ b/packages/graphiql-react/package.json @@ -32,8 +32,8 @@ }, "peerDependencies": { "graphql": "^15.5.0 || ^16.0.0", - "react": "^16.8.0 || ^17.0.0 || ^18.0.0", - "react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0" + "react": "^18.2.0", + "react-dom": "^18.2.0" }, "dependencies": { "@radix-ui/react-dialog": "^1.0.3", @@ -41,7 +41,7 @@ "@radix-ui/react-tooltip": "^1.0.5", "@radix-ui/react-dropdown-menu": "^2.0.4", "@graphiql/toolkit": "^0.8.4", - "@reach/combobox": "^0.17.0", + "cmdk": "^0.2.0", "clsx": "^1.2.1", "codemirror": "^5.65.3", "codemirror-graphql": "^2.0.8", @@ -58,8 +58,8 @@ "@vitejs/plugin-react": "^1.3.0", "graphql": "^16.4.0", "postcss-nesting": "^10.1.7", - "react": "^17.0.2", - "react-dom": "^17.0.2", + "react": "^18.2.0", + "react-dom": "^18.2.0", "typescript": "^4.6.3", "vite": "^2.9.13", "vite-plugin-react-svg": "^0.2.0" diff --git a/packages/graphiql-react/src/explorer/components/doc-explorer.css b/packages/graphiql-react/src/explorer/components/doc-explorer.css index b5b081b6b39..1bc57f94bb7 100644 --- a/packages/graphiql-react/src/explorer/components/doc-explorer.css +++ b/packages/graphiql-react/src/explorer/components/doc-explorer.css @@ -37,12 +37,12 @@ left: 0; } - & [data-reach-combobox-input] { + & [cmdk-input] { height: 24px; width: 4ch; } - & [data-reach-combobox-input]:focus { + & [cmdk-input]:focus { width: 100%; } } diff --git a/packages/graphiql-react/src/explorer/components/search.css b/packages/graphiql-react/src/explorer/components/search.css index 624b2b6f094..878ad84c1a1 100644 --- a/packages/graphiql-react/src/explorer/components/search.css +++ b/packages/graphiql-react/src/explorer/components/search.css @@ -1,6 +1,4 @@ -@import url('@reach/combobox/styles.css'); - -[data-reach-combobox] { +[cmdk-root] { color: hsla(var(--color-neutral), var(--alpha-secondary)); &:not([data-state='idle']) { @@ -25,7 +23,7 @@ padding: var(--px-8) var(--px-12); } -[data-reach-combobox-input] { +[cmdk-input] { border: none; background-color: transparent; margin-left: var(--px-4); @@ -36,7 +34,7 @@ } } -[data-reach-combobox-popover] { +[cmdk-list] { background-color: hsl(var(--color-base)); border: none; border-bottom-left-radius: var(--border-radius-4); @@ -53,12 +51,12 @@ position: relative; } -[data-reach-combobox-list] { +[cmdk-group-items] { font-size: var(--font-size-body); padding: var(--px-4); } -[data-reach-combobox-option] { +[cmdk-item] { border-radius: var(--border-radius-4); color: hsla(var(--color-neutral), var(--alpha-secondary)); overflow-x: hidden; @@ -66,7 +64,7 @@ text-overflow: ellipsis; white-space: nowrap; - &[data-highlighted] { + &[aria-selected='true'] { background-color: hsla(var(--color-neutral), var(--alpha-background-light)); } @@ -77,7 +75,7 @@ ); } - &[data-highlighted]:hover { + &[aria-selected='true']:hover { background-color: hsla(var(--color-neutral), var(--alpha-background-heavy)); } diff --git a/packages/graphiql-react/src/explorer/components/search.tsx b/packages/graphiql-react/src/explorer/components/search.tsx index 886899d7b89..9aa6eefcd3b 100644 --- a/packages/graphiql-react/src/explorer/components/search.tsx +++ b/packages/graphiql-react/src/explorer/components/search.tsx @@ -1,10 +1,3 @@ -import { - Combobox, - ComboboxInput, - ComboboxPopover, - ComboboxList, - ComboboxOption, -} from '@reach/combobox'; import { GraphQLArgument, GraphQLField, @@ -15,6 +8,7 @@ import { isObjectType, } from 'graphql'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { Command } from 'cmdk'; import { MagnifyingGlassIcon } from '../../icons'; import { useSchemaContext } from '../../schema'; import debounce from '../../utility/debounce'; @@ -31,8 +25,6 @@ export function Search() { }); const inputRef = useRef(null); - const popoverRef = useRef(null); - const getSearchResults = useSearchResults(); const [searchValue, setSearchValue] = useState(''); @@ -61,124 +53,99 @@ export function Search() { const navItem = explorerNavStack.at(-1)!; + const onSelect = useCallback( + (value: string) => { + const def = JSON.parse(value) as TypeMatch | FieldMatch; + push( + 'field' in def + ? { name: def.field.name, def: def.field } + : { name: def.type.name, def: def.type }, + ); + }, + [push], + ); + const [isFocused, setIsFocused] = useState(false); + const handleFocus = useCallback(e => { + setIsFocused(e.type === 'focus'); + }, []); + const shouldSearchBoxAppear = explorerNavStack.length === 1 || isObjectType(navItem.def) || isInterfaceType(navItem.def) || isInputObjectType(navItem.def); - - return shouldSearchBoxAppear ? ( - { - const def = value as unknown as TypeMatch | FieldMatch; - push( - 'field' in def - ? { name: def.field.name, def: def.field } - : { name: def.type.name, def: def.type }, - ); - }} + if (!shouldSearchBoxAppear) { + return null; + } + return ( +
{ - if (inputRef.current) { - inputRef.current.focus(); - } + inputRef.current?.focus(); }} > - { - setSearchValue(event.target.value); - }} - onKeyDown={event => { - if (!event.isDefaultPrevented()) { - const container = popoverRef.current; - if (!container) { - return; - } - - window.requestAnimationFrame(() => { - const element = container.querySelector('[aria-selected=true]'); - if (!(element instanceof HTMLElement)) { - return; - } - const top = element.offsetTop - container.scrollTop; - const bottom = - container.scrollTop + - container.clientHeight - - (element.offsetTop + element.clientHeight); - if (bottom < 0) { - container.scrollTop -= bottom; - } - if (top < 0) { - container.scrollTop += top; - } - }); - } - - // We don't want for example "Escape" key presses to bubble up - // further. This could have other effects like closing a dialog - // that contains this component. - event.stopPropagation(); - }} +
- - - {/** - * Setting the `index` prop explicitly on the `ComboboxOption` solves - * buggy behavior of the internal ordering of the combobox items. - * (Sometimes this results in weird jumps when using the keyboard to - * navigate search results.) - */} - {results.within.map((result, i) => ( - - - - ))} - {results.within.length > 0 && - results.types.length + results.fields.length > 0 ? ( -
- Other results -
- ) : null} - {results.types.map((result, i) => ( - - - - ))} - {results.fields.map((result, i) => ( - - . - - - ))} - {results.within.length + - results.types.length + - results.fields.length === - 0 ? ( -
- No results found -
- ) : null} - - - - ) : null; + + {isFocused && ( + + + No results found + + + + {results.within.map((result, i) => ( + + + + ))} + {results.within.length > 0 && + results.types.length + results.fields.length > 0 ? ( +
+ Other results +
+ ) : null} + {results.types.map((result, i) => ( + + + + ))} + {results.fields.map((result, i) => ( + + . + + + ))} + + + )} + + ); } type TypeMatch = { type: GraphQLNamedType }; @@ -283,7 +250,7 @@ export function useSearchResults(caller?: Function) { ); } -function isMatch(sourceText: string, searchValue: string) { +function isMatch(sourceText: string, searchValue: string): boolean { try { const escaped = searchValue.replaceAll(/[^_0-9A-Za-z]/g, ch => '\\' + ch); return sourceText.search(new RegExp(escaped, 'i')) !== -1; @@ -305,20 +272,18 @@ type FieldProps = { argument?: GraphQLArgument; }; -function Field(props: FieldProps) { +function Field({ field, argument }: FieldProps) { return ( <> - - {props.field.name} - - {props.argument ? ( + {field.name} + {argument ? ( <> ( - {props.argument.name} + {argument.name} :{' '} - {renderType(props.argument.type, namedType => ( + {renderType(argument.type, namedType => ( ))} ) diff --git a/packages/graphiql-react/src/toolbar/menu.tsx b/packages/graphiql-react/src/toolbar/menu.tsx index 83d45976c5e..a3b96be1efb 100644 --- a/packages/graphiql-react/src/toolbar/menu.tsx +++ b/packages/graphiql-react/src/toolbar/menu.tsx @@ -14,6 +14,7 @@ const ToolbarMenuRoot = forwardRef< HTMLDivElement, ToolbarMenuProps & JSX.IntrinsicElements['div'] >(({ button, children, label, ...props }, ref) => ( + // @ts-expect-error -- Should I remove ref? got Property 'ref' does not exist on type 'IntrinsicAttributes & DropdownMenuProps & { children?: ReactNode; }' { describe('GraphiQL DocExplorer - search', () => { beforeEach(() => { cy.get('.graphiql-sidebar button').eq(0).click(); - cy.get('[data-reach-combobox-input]').type('test'); - cy.get('[data-reach-combobox-option]').should('have.length', 7); + cy.get('[cmdk-input]').type('test'); + cy.get('[cmdk-item]').should('have.length', 7); }); it('Searches docs for values', () => { - cy.get('[data-reach-combobox-popover]').should('not.have.attr', 'hidden'); + cy.get('[cmdk-list]').should('not.have.attr', 'hidden'); }); it('Navigates to a docs entry on selecting a search result', () => { - cy.get('[data-reach-combobox-option]').eq(4).children().click(); + cy.get('[cmdk-item]').eq(4).children().click(); cy.get('.graphiql-doc-explorer-title').should('have.text', 'TestInput'); }); it('Allows searching fields within a type', () => { - cy.get('[data-reach-combobox-option]').eq(4).children().click(); - cy.get('[data-reach-combobox-input]').type('list'); - cy.get('[data-reach-combobox-option]').should('have.length', 14); + cy.get('[cmdk-item]').eq(4).children().click(); + cy.get('[cmdk-input]').type('list'); + cy.get('[cmdk-item]').should('have.length', 14); cy.get( - '[data-reach-combobox-popover] .graphiql-doc-explorer-search-divider', + '[cmdk-list] .graphiql-doc-explorer-search-divider', ).should('have.text', 'Other results'); - cy.get('[data-reach-combobox-option]').contains('hasArgs'); + cy.get('[cmdk-item]').contains('hasArgs'); }); it('Closes popover when blurring input', () => { - cy.get('[data-reach-combobox-input]').blur(); - cy.get('[data-reach-combobox-popover]').should('have.attr', 'hidden'); + cy.get('[cmdk-input]').blur(); + cy.get('[cmdk-list]').should('have.attr', 'hidden'); }); it('Navigates back', () => { - cy.get('[data-reach-combobox-option]').eq(4).children().click(); + cy.get('[cmdk-item]').eq(4).children().click(); cy.get('.graphiql-doc-explorer-back').click(); cy.get('.graphiql-doc-explorer-title').should('have.text', 'Docs'); }); it('Type fields link to their own docs entry', () => { - cy.get('[data-reach-combobox-option]').last().click(); + cy.get('[cmdk-item]').last().click(); cy.get('.graphiql-doc-explorer-title').should('have.text', 'isTest'); cy.get('.graphiql-markdown-description').should( 'have.text', diff --git a/packages/graphiql/package.json b/packages/graphiql/package.json index 3235596025e..f2ee725582e 100644 --- a/packages/graphiql/package.json +++ b/packages/graphiql/package.json @@ -57,8 +57,8 @@ }, "peerDependencies": { "graphql": "^15.5.0 || ^16.0.0", - "react": "^16.8.0 || ^17.0.0 || ^18.0.0", - "react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0" + "react": "^18.2.0", + "react-dom": "^18.2.0" }, "devDependencies": { "@cypress/webpack-preprocessor": "^5.5.0", @@ -87,8 +87,8 @@ "postcss-import": "15.1.0", "postcss-preset-env": "^8.0.1", "prop-types": "15.7.2", - "react": "^17.0.2", - "react-dom": "^17.0.2", + "react": "^18.2.0", + "react-dom": "^18.2.0", "react-hot-loader": "^4.12.20", "react-test-renderer": "^16.13.1", "require-context.macro": "^1.2.2", diff --git a/packages/graphiql/resources/index.html.ejs b/packages/graphiql/resources/index.html.ejs index 9335da7ec34..f3512006ed6 100644 --- a/packages/graphiql/resources/index.html.ejs +++ b/packages/graphiql/resources/index.html.ejs @@ -28,16 +28,8 @@ If you do not want to rely on a CDN, you can host these files locally or include them directly in your favored resource bundler. --> - - + +