From 87d73e10d8fbcfc3d0e4ce97d3365ac3ad39b2ca Mon Sep 17 00:00:00 2001 From: David Szabo Date: Thu, 26 Nov 2020 11:43:17 +0100 Subject: [PATCH 01/28] Try context --- .../reakit-system/src/StateContextHoc.tsx | 35 +++++++++ packages/reakit-system/src/createComponent.ts | 57 ++++++++++++-- .../reakit/src/Combobox/ComboboxOption.ts | 2 + .../reakit/src/Combobox/ComboboxPopover.ts | 5 ++ .../ComboboxNonContextVsContext/index.tsx | 78 +++++++++++++++++++ .../ComboboxNonContextVsContext/style.css | 1 + .../src/Combobox/__examples__/index.story.ts | 1 + 7 files changed, 174 insertions(+), 5 deletions(-) create mode 100644 packages/reakit-system/src/StateContextHoc.tsx create mode 100644 packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx create mode 100644 packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/style.css diff --git a/packages/reakit-system/src/StateContextHoc.tsx b/packages/reakit-system/src/StateContextHoc.tsx new file mode 100644 index 00000000..d6c0a872 --- /dev/null +++ b/packages/reakit-system/src/StateContextHoc.tsx @@ -0,0 +1,35 @@ +import React from "react"; +import { unstable_useId as useId } from "reakit/Id"; + +export const StateContextHoc = (Comp, ctx) => (props) => { + const context = React.useContext(ctx); + + let subscribe; + let initialState; + if (context) { + const { subscribe: _subscribe, ..._initialState } = context; + subscribe = _subscribe; + initialState = _initialState; + } + + const { id } = useId({ ...initialState, ...props }); + const [state, setState] = React.useState(initialState); + + React.useEffect( + () => + subscribe + ? subscribe((nextState) => { + if ( + state.currentId === null || + id === state.currentId || + id === nextState.currentId + ) { + setState(nextState); + } + }) + : undefined, + [subscribe, state?.currentId, id, props.id] + ); + + return ; +}; diff --git a/packages/reakit-system/src/createComponent.ts b/packages/reakit-system/src/createComponent.ts index 5dc98e35..16a58b50 100644 --- a/packages/reakit-system/src/createComponent.ts +++ b/packages/reakit-system/src/createComponent.ts @@ -6,6 +6,7 @@ import { normalizePropsAreEqual } from "reakit-utils/normalizePropsAreEqual"; import { forwardRef } from "./__utils/forwardRef"; import { useCreateElement as defaultUseCreateElement } from "./useCreateElement"; import { memo } from "./__utils/memo"; +import { StateContextHoc } from "./StateContextHoc"; type RoleHTMLProps = React.HTMLAttributes & React.RefAttributes & { @@ -18,11 +19,13 @@ type Hook = { __keys?: ReadonlyArray; }; -type Options = { +type Options = { as: T; useHook?: Hook; keys?: ReadonlyArray; memo?: boolean; + context?: React.Context; + isContextProvider?: boolean; propsAreEqual?: (prev: O, next: O) => boolean; useCreateElement?: ( type: T, @@ -56,6 +59,8 @@ export function createComponent({ as: type, useHook, memo: shouldMemo, + context, + isContextProvider = false, propsAreEqual = useHook?.unstable_propsAreEqual, keys = useHook?.__keys || [], useCreateElement = defaultUseCreateElement, @@ -64,8 +69,13 @@ export function createComponent({ { as = type, ...props }: PropsWithAs, ref: React.Ref ) => { + const initialState = React.useRef(); + if (useHook) { const [options, htmlProps] = splitProps(props, keys); + if (!initialState.current) { + initialState.current = options; + } const { wrapElement, ...elementProps } = useHook(options, { ref, ...htmlProps, @@ -73,10 +83,43 @@ export function createComponent({ // @ts-ignore const asKeys = as.render?.__keys || as.__keys; const asOptions = asKeys && splitProps(props, asKeys)[0]; - const allProps = asOptions - ? { ...elementProps, ...asOptions } - : elementProps; - const element = useCreateElement(as, allProps as typeof props); + + const allProps = + // If `as` is a string, then that means it's an element not a component + // we don't need to pass the state in this case. + // If a component is passed, then the component can decide + // what should happen with the state prop. + isPlainObject(props.state) && typeof as !== "string" + ? { state: props.state, ...elementProps } + : asOptions + ? { ...elementProps, ...asOptions } + : elementProps; + + let element = useCreateElement(as, allProps as typeof props); + if (context && isContextProvider) { + if (isContextProvider) { + const listenersRef = React.useRef(new Set()); + + const subscribe = React.useCallback((listener) => { + listenersRef.current.add(listener); + return () => listenersRef.current.delete(listener); + }, []); + + React.useEffect(() => { + for (const listener of listenersRef.current) { + listener(options); + } + }, [options]); + + const value = React.useMemo( + () => ({ ...initialState.current, subscribe }), + [initialState.current, subscribe] + ); + + element = React.createElement(context.Provider, { value }, element); + } + } + if (wrapElement) { return wrapElement(element); } @@ -91,6 +134,10 @@ export function createComponent({ Comp = forwardRef(Comp); + if (context && !isContextProvider) { + Comp = StateContextHoc(Comp, context); + } + if (shouldMemo) { Comp = memo(Comp, propsAreEqual && normalizePropsAreEqual(propsAreEqual)); } diff --git a/packages/reakit/src/Combobox/ComboboxOption.ts b/packages/reakit/src/Combobox/ComboboxOption.ts index 4a9ef312..eb908776 100644 --- a/packages/reakit/src/Combobox/ComboboxOption.ts +++ b/packages/reakit/src/Combobox/ComboboxOption.ts @@ -11,6 +11,7 @@ import { unstable_ComboboxItemHTMLProps as ComboboxItemHTMLProps, unstable_useComboboxItem as useComboboxItem, } from "./ComboboxItem"; +import { Context } from "./ComboboxPopover"; export const unstable_useComboboxOption = createHook< unstable_ComboboxOptionOptions, @@ -29,6 +30,7 @@ export const unstable_ComboboxOption = createComponent({ as: "div", memo: true, useHook: unstable_useComboboxOption, + context: Context, }); export type unstable_ComboboxOptionOptions = CompositeItemOptions & diff --git a/packages/reakit/src/Combobox/ComboboxPopover.ts b/packages/reakit/src/Combobox/ComboboxPopover.ts index 835d508b..46069a3c 100644 --- a/packages/reakit/src/Combobox/ComboboxPopover.ts +++ b/packages/reakit/src/Combobox/ComboboxPopover.ts @@ -1,3 +1,4 @@ +import React from "react"; import { createComponent } from "reakit-system/createComponent"; import { createHook } from "reakit-system/createHook"; import { useWarning } from "reakit-warning"; @@ -42,9 +43,13 @@ export const unstable_useComboboxPopover = createHook< }, }); +export const Context = React.createContext({}); + export const unstable_ComboboxPopover = createComponent({ as: "div", useHook: unstable_useComboboxPopover, + context: Context, + isContextProvider: true, useCreateElement: (type, props, children) => { useWarning( !props["aria-label"] && !props["aria-labelledby"], diff --git a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx new file mode 100644 index 00000000..9765406e --- /dev/null +++ b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx @@ -0,0 +1,78 @@ +import { + unstable_Combobox as Combobox, + unstable_ComboboxOption as ComboboxOption, + unstable_useComboboxState as useComboboxState, + unstable_ComboboxPopover as ComboboxPopover, +} from "reakit/Combobox"; +import "./style.css"; +import * as React from "react"; + +function ComboboxOriginal() { + const items = Array.from({ length: 250 }).map((_, i) => `Item ${i}`); + + const combobox = useComboboxState({ + values: items, + gutter: 8, + limit: false, + autoSelect: true, + }); + + return ( + <> + + + {items.map((value) => ( + + ))} + + + ); +} + +function ComboboxContext() { + const items = Array.from({ length: 250 }).map((_, i) => `Item ${i}`); + + const combobox = useComboboxState({ + values: items, + gutter: 8, + limit: false, + autoSelect: true, + }); + + return ( + <> + + + {items.map((value) => ( + + ))} + + + ); +} + +export default function ComboBoxNonContextVsContext() { + return ( + <> +
+

+ Original Combobox, clicking on it may take some time. Use arrows (hold + down your down-arrow key) to navigate between elements. You will + notice how slow it is. +

+ + +
+ +
+

+ New Combobox using Context, clicking on it takes less time. Use arrows + (hold down your down-arrow key) to navigate between elements. You will + notice how fast it is compared to the original. +

+ + +
+ + ); +} diff --git a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/style.css b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/style.css new file mode 100644 index 00000000..3ffb28f1 --- /dev/null +++ b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/style.css @@ -0,0 +1 @@ +@import "../AccessibleCombobox/style.css"; diff --git a/packages/reakit/src/Combobox/__examples__/index.story.ts b/packages/reakit/src/Combobox/__examples__/index.story.ts index acb37f60..d9907ee8 100644 --- a/packages/reakit/src/Combobox/__examples__/index.story.ts +++ b/packages/reakit/src/Combobox/__examples__/index.story.ts @@ -11,6 +11,7 @@ export { default as ComboboxList } from "./ComboboxList"; export { default as ComboboxListGridWithPopover } from "./ComboboxListGridWithPopover"; export { default as ComboboxMinValueLength } from "./ComboboxMinValueLength"; export { default as ComboboxVisible } from "./ComboboxVisible"; +export { default as ComboboxNonContextVsContext } from "./ComboboxNonContextVsContext"; export default { title: "Combobox", From f26d9291f20c4250105357233dd04d3af0d24eb8 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Thu, 26 Nov 2020 11:45:31 +0100 Subject: [PATCH 02/28] Styling --- .../ComboboxNonContextVsContext/index.tsx | 12 ++++++------ .../ComboboxNonContextVsContext/style.css | 9 +++++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx index 9765406e..b20326be 100644 --- a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx +++ b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx @@ -56,9 +56,9 @@ export default function ComboBoxNonContextVsContext() { <>

- Original Combobox, clicking on it may take some time. Use arrows (hold - down your down-arrow key) to navigate between elements. You will - notice how slow it is. + Original Combobox, clicking on it may take some time. + Use arrows (hold down your down-arrow key) to navigate between + elements. You will notice how slow it is.

@@ -66,9 +66,9 @@ export default function ComboBoxNonContextVsContext() {

- New Combobox using Context, clicking on it takes less time. Use arrows - (hold down your down-arrow key) to navigate between elements. You will - notice how fast it is compared to the original. + New Combobox using Context, clicking on it takes less + time. Use arrows (hold down your down-arrow key) to navigate between + elements. You will notice how fast it is compared to the original.

diff --git a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/style.css b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/style.css index 3ffb28f1..c317b3aa 100644 --- a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/style.css +++ b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/style.css @@ -1 +1,10 @@ @import "../AccessibleCombobox/style.css"; + +#combobox--combobox-non-context-vs-context { + display: flex; +} + +#combobox--combobox-non-context-vs-context > div { + flex: 1; + padding: 1rem; +} From b39e552a9f80111c32b37798ac7dfff6a657c2c4 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 10:47:38 +0100 Subject: [PATCH 03/28] Typing --- .../reakit-system/src/StateContextHoc.tsx | 71 +++++++++++-------- packages/reakit-system/src/createComponent.ts | 30 +++++--- 2 files changed, 62 insertions(+), 39 deletions(-) diff --git a/packages/reakit-system/src/StateContextHoc.tsx b/packages/reakit-system/src/StateContextHoc.tsx index d6c0a872..fbd00498 100644 --- a/packages/reakit-system/src/StateContextHoc.tsx +++ b/packages/reakit-system/src/StateContextHoc.tsx @@ -1,35 +1,50 @@ import React from "react"; import { unstable_useId as useId } from "reakit/Id"; -export const StateContextHoc = (Comp, ctx) => (props) => { - const context = React.useContext(ctx); +export type StateContext = React.Context<{ + initialState: O; + subscribe: StateContextSubscribe; +}>; +export type StateContextListener = (options: O) => void; +export type StateContextSubscribe = (callback: (options: O) => any) => any; +export type HOCInner = (props: O) => T; - let subscribe; - let initialState; - if (context) { - const { subscribe: _subscribe, ..._initialState } = context; - subscribe = _subscribe; - initialState = _initialState; - } +export function StateContextHoc< + T extends React.ComponentType, + O extends React.HTMLAttributes +>(Comp: T, ctx: StateContext): HOCInner { + const inner: HOCInner = (props: O) => { + const context = React.useContext(ctx); - const { id } = useId({ ...initialState, ...props }); - const [state, setState] = React.useState(initialState); + let subscribe: StateContextSubscribe | null = null; + let initialState: O = {} as O; + if (context) { + const { subscribe: _subscribe, initialState: _initialState } = context; + subscribe = _subscribe; + initialState = _initialState; + } - React.useEffect( - () => - subscribe - ? subscribe((nextState) => { - if ( - state.currentId === null || - id === state.currentId || - id === nextState.currentId - ) { - setState(nextState); - } - }) - : undefined, - [subscribe, state?.currentId, id, props.id] - ); + const { id } = useId({ ...initialState, ...props }); + const [state, setState] = React.useState(initialState); - return ; -}; + React.useEffect( + () => + subscribe + ? subscribe((nextState: any) => { + if ( + state.currentId === null || + id === state.currentId || + id === nextState.currentId + ) { + setState(nextState); + } + }) + : undefined, + [subscribe, state?.currentId, id, props.id] + ); + + return (() as unknown) as T; + }; + + return inner; +} diff --git a/packages/reakit-system/src/createComponent.ts b/packages/reakit-system/src/createComponent.ts index 16a58b50..cdf535b8 100644 --- a/packages/reakit-system/src/createComponent.ts +++ b/packages/reakit-system/src/createComponent.ts @@ -6,7 +6,12 @@ import { normalizePropsAreEqual } from "reakit-utils/normalizePropsAreEqual"; import { forwardRef } from "./__utils/forwardRef"; import { useCreateElement as defaultUseCreateElement } from "./useCreateElement"; import { memo } from "./__utils/memo"; -import { StateContextHoc } from "./StateContextHoc"; +import { + StateContext, + StateContextHoc, + StateContextListener, + StateContextSubscribe, +} from "./StateContextHoc"; type RoleHTMLProps = React.HTMLAttributes & React.RefAttributes & { @@ -19,12 +24,12 @@ type Hook = { __keys?: ReadonlyArray; }; -type Options = { +type Options = { as: T; useHook?: Hook; keys?: ReadonlyArray; memo?: boolean; - context?: React.Context; + context?: StateContext; isContextProvider?: boolean; propsAreEqual?: (prev: O, next: O) => boolean; useCreateElement?: ( @@ -69,7 +74,7 @@ export function createComponent({ { as = type, ...props }: PropsWithAs, ref: React.Ref ) => { - const initialState = React.useRef(); + const initialState = React.useRef(); if (useHook) { const [options, htmlProps] = splitProps(props, keys); @@ -98,12 +103,15 @@ export function createComponent({ let element = useCreateElement(as, allProps as typeof props); if (context && isContextProvider) { if (isContextProvider) { - const listenersRef = React.useRef(new Set()); - - const subscribe = React.useCallback((listener) => { - listenersRef.current.add(listener); - return () => listenersRef.current.delete(listener); - }, []); + const listenersRef = React.useRef(new Set>()); + + const subscribe: StateContextSubscribe = React.useCallback( + (listener: StateContextListener) => { + listenersRef.current.add(listener); + return () => listenersRef.current.delete(listener); + }, + [] + ); React.useEffect(() => { for (const listener of listenersRef.current) { @@ -112,7 +120,7 @@ export function createComponent({ }, [options]); const value = React.useMemo( - () => ({ ...initialState.current, subscribe }), + () => ({ initialState: initialState.current as O, subscribe }), [initialState.current, subscribe] ); From 5d20394cead1b2996336a3e8f47a57641b149b44 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 10:54:06 +0100 Subject: [PATCH 04/28] Add createStateContext --- packages/reakit-system/src/StateContextHoc.tsx | 5 +++-- packages/reakit-system/src/createStateContext.ts | 9 +++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 packages/reakit-system/src/createStateContext.ts diff --git a/packages/reakit-system/src/StateContextHoc.tsx b/packages/reakit-system/src/StateContextHoc.tsx index fbd00498..9133dacf 100644 --- a/packages/reakit-system/src/StateContextHoc.tsx +++ b/packages/reakit-system/src/StateContextHoc.tsx @@ -1,10 +1,11 @@ import React from "react"; import { unstable_useId as useId } from "reakit/Id"; -export type StateContext = React.Context<{ +export type StateContext = React.Context>; +export type StateContextValue = { initialState: O; subscribe: StateContextSubscribe; -}>; +}; export type StateContextListener = (options: O) => void; export type StateContextSubscribe = (callback: (options: O) => any) => any; export type HOCInner = (props: O) => T; diff --git a/packages/reakit-system/src/createStateContext.ts b/packages/reakit-system/src/createStateContext.ts new file mode 100644 index 00000000..07f2f0d3 --- /dev/null +++ b/packages/reakit-system/src/createStateContext.ts @@ -0,0 +1,9 @@ +import React from "react"; +import { StateContext } from "./StateContextHoc"; + +export function createStateContext(): StateContext { + return (React.createContext({ + initialState: {}, + subscribe: () => {}, + }) as unknown) as StateContext; +} From 3025ac7fc85b8b103e54d73cf831db9c69371b46 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 11:04:52 +0100 Subject: [PATCH 05/28] Remove HOCInner --- packages/reakit-system/src/StateContextHoc.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/reakit-system/src/StateContextHoc.tsx b/packages/reakit-system/src/StateContextHoc.tsx index 9133dacf..9ddcb924 100644 --- a/packages/reakit-system/src/StateContextHoc.tsx +++ b/packages/reakit-system/src/StateContextHoc.tsx @@ -8,13 +8,12 @@ export type StateContextValue = { }; export type StateContextListener = (options: O) => void; export type StateContextSubscribe = (callback: (options: O) => any) => any; -export type HOCInner = (props: O) => T; export function StateContextHoc< T extends React.ComponentType, O extends React.HTMLAttributes ->(Comp: T, ctx: StateContext): HOCInner { - const inner: HOCInner = (props: O) => { +>(Comp: T, ctx: StateContext): T { + const inner = (props: O) => { const context = React.useContext(ctx); let subscribe: StateContextSubscribe | null = null; @@ -44,8 +43,8 @@ export function StateContextHoc< [subscribe, state?.currentId, id, props.id] ); - return (() as unknown) as T; + return ; }; - return inner; + return (inner as unknown) as T; } From d86f53ab149651e784d4d425605cde18c77fbc91 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 11:05:06 +0100 Subject: [PATCH 06/28] Use createStateContext --- packages/reakit-system/src/index.ts | 1 + packages/reakit/src/Combobox/ComboboxOption.ts | 4 ++-- packages/reakit/src/Combobox/ComboboxPopover.ts | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/reakit-system/src/index.ts b/packages/reakit-system/src/index.ts index 2370206d..5a36f9af 100644 --- a/packages/reakit-system/src/index.ts +++ b/packages/reakit-system/src/index.ts @@ -1,5 +1,6 @@ export * from "./createComponent"; export * from "./createHook"; +export * from "./createStateContext"; export * from "./mergeSystem"; export * from "./SystemContext"; export * from "./SystemProvider"; diff --git a/packages/reakit/src/Combobox/ComboboxOption.ts b/packages/reakit/src/Combobox/ComboboxOption.ts index eb908776..81fae9d3 100644 --- a/packages/reakit/src/Combobox/ComboboxOption.ts +++ b/packages/reakit/src/Combobox/ComboboxOption.ts @@ -11,7 +11,7 @@ import { unstable_ComboboxItemHTMLProps as ComboboxItemHTMLProps, unstable_useComboboxItem as useComboboxItem, } from "./ComboboxItem"; -import { Context } from "./ComboboxPopover"; +import { StateContext } from "./ComboboxPopover"; export const unstable_useComboboxOption = createHook< unstable_ComboboxOptionOptions, @@ -30,7 +30,7 @@ export const unstable_ComboboxOption = createComponent({ as: "div", memo: true, useHook: unstable_useComboboxOption, - context: Context, + context: StateContext, }); export type unstable_ComboboxOptionOptions = CompositeItemOptions & diff --git a/packages/reakit/src/Combobox/ComboboxPopover.ts b/packages/reakit/src/Combobox/ComboboxPopover.ts index 46069a3c..6b8a4be2 100644 --- a/packages/reakit/src/Combobox/ComboboxPopover.ts +++ b/packages/reakit/src/Combobox/ComboboxPopover.ts @@ -1,8 +1,8 @@ -import React from "react"; import { createComponent } from "reakit-system/createComponent"; import { createHook } from "reakit-system/createHook"; import { useWarning } from "reakit-warning"; import { useCreateElement } from "reakit-system/useCreateElement"; +import { createStateContext } from "reakit-system/createStateContext"; import { PopoverOptions, PopoverHTMLProps, @@ -43,12 +43,12 @@ export const unstable_useComboboxPopover = createHook< }, }); -export const Context = React.createContext({}); +export const StateContext = createStateContext(); export const unstable_ComboboxPopover = createComponent({ as: "div", useHook: unstable_useComboboxPopover, - context: Context, + context: StateContext, isContextProvider: true, useCreateElement: (type, props, children) => { useWarning( From 7544049847baeb6367784572d2281f50ed4db7c1 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 11:11:09 +0100 Subject: [PATCH 07/28] Rename hoc to withStateContextSubscriber --- packages/reakit-system/src/createComponent.ts | 6 +++--- packages/reakit-system/src/createStateContext.ts | 2 +- .../{StateContextHoc.tsx => withStateContextSubscriber.tsx} | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename packages/reakit-system/src/{StateContextHoc.tsx => withStateContextSubscriber.tsx} (97%) diff --git a/packages/reakit-system/src/createComponent.ts b/packages/reakit-system/src/createComponent.ts index cdf535b8..0d682bb2 100644 --- a/packages/reakit-system/src/createComponent.ts +++ b/packages/reakit-system/src/createComponent.ts @@ -8,10 +8,10 @@ import { useCreateElement as defaultUseCreateElement } from "./useCreateElement" import { memo } from "./__utils/memo"; import { StateContext, - StateContextHoc, + withStateContextSubscriber, StateContextListener, StateContextSubscribe, -} from "./StateContextHoc"; +} from "./withStateContextSubscriber"; type RoleHTMLProps = React.HTMLAttributes & React.RefAttributes & { @@ -143,7 +143,7 @@ export function createComponent({ Comp = forwardRef(Comp); if (context && !isContextProvider) { - Comp = StateContextHoc(Comp, context); + Comp = withStateContextSubscriber(Comp, context); } if (shouldMemo) { diff --git a/packages/reakit-system/src/createStateContext.ts b/packages/reakit-system/src/createStateContext.ts index 07f2f0d3..27143e10 100644 --- a/packages/reakit-system/src/createStateContext.ts +++ b/packages/reakit-system/src/createStateContext.ts @@ -1,5 +1,5 @@ import React from "react"; -import { StateContext } from "./StateContextHoc"; +import { StateContext } from "./withStateContextSubscriber"; export function createStateContext(): StateContext { return (React.createContext({ diff --git a/packages/reakit-system/src/StateContextHoc.tsx b/packages/reakit-system/src/withStateContextSubscriber.tsx similarity index 97% rename from packages/reakit-system/src/StateContextHoc.tsx rename to packages/reakit-system/src/withStateContextSubscriber.tsx index 9ddcb924..b744d762 100644 --- a/packages/reakit-system/src/StateContextHoc.tsx +++ b/packages/reakit-system/src/withStateContextSubscriber.tsx @@ -9,7 +9,7 @@ export type StateContextValue = { export type StateContextListener = (options: O) => void; export type StateContextSubscribe = (callback: (options: O) => any) => any; -export function StateContextHoc< +export function withStateContextSubscriber< T extends React.ComponentType, O extends React.HTMLAttributes >(Comp: T, ctx: StateContext): T { From 65acc9c27fddfd5eddf82fd391288a9bc0a5a048 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 12:43:15 +0100 Subject: [PATCH 08/28] Fix search in story --- .../__examples__/ComboboxNonContextVsContext/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx index b20326be..420e9893 100644 --- a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx +++ b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx @@ -21,7 +21,7 @@ function ComboboxOriginal() { <> - {items.map((value) => ( + {combobox.matches.map((value) => ( ))} @@ -43,7 +43,7 @@ function ComboboxContext() { <> - {items.map((value) => ( + {combobox.matches.map((value) => ( ))} From 0ab12372ecdcae06007d7c6ce4bb4d06a002b766 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 12:45:41 +0100 Subject: [PATCH 09/28] Try use hook based context --- packages/reakit-system/src/createComponent.ts | 46 +--------- packages/reakit-system/src/useStateContext.ts | 6 ++ .../src/withStateContextSubscriber.tsx | 84 +++++++++++++++++++ .../reakit/src/Combobox/ComboboxOption.ts | 8 +- .../reakit/src/Combobox/ComboboxPopover.ts | 10 +-- 5 files changed, 102 insertions(+), 52 deletions(-) create mode 100644 packages/reakit-system/src/useStateContext.ts diff --git a/packages/reakit-system/src/createComponent.ts b/packages/reakit-system/src/createComponent.ts index 0d682bb2..7d1e80bf 100644 --- a/packages/reakit-system/src/createComponent.ts +++ b/packages/reakit-system/src/createComponent.ts @@ -6,12 +6,6 @@ import { normalizePropsAreEqual } from "reakit-utils/normalizePropsAreEqual"; import { forwardRef } from "./__utils/forwardRef"; import { useCreateElement as defaultUseCreateElement } from "./useCreateElement"; import { memo } from "./__utils/memo"; -import { - StateContext, - withStateContextSubscriber, - StateContextListener, - StateContextSubscribe, -} from "./withStateContextSubscriber"; type RoleHTMLProps = React.HTMLAttributes & React.RefAttributes & { @@ -29,8 +23,6 @@ type Options = { useHook?: Hook; keys?: ReadonlyArray; memo?: boolean; - context?: StateContext; - isContextProvider?: boolean; propsAreEqual?: (prev: O, next: O) => boolean; useCreateElement?: ( type: T, @@ -74,13 +66,8 @@ export function createComponent({ { as = type, ...props }: PropsWithAs, ref: React.Ref ) => { - const initialState = React.useRef(); - if (useHook) { const [options, htmlProps] = splitProps(props, keys); - if (!initialState.current) { - initialState.current = options; - } const { wrapElement, ...elementProps } = useHook(options, { ref, ...htmlProps, @@ -100,34 +87,7 @@ export function createComponent({ ? { ...elementProps, ...asOptions } : elementProps; - let element = useCreateElement(as, allProps as typeof props); - if (context && isContextProvider) { - if (isContextProvider) { - const listenersRef = React.useRef(new Set>()); - - const subscribe: StateContextSubscribe = React.useCallback( - (listener: StateContextListener) => { - listenersRef.current.add(listener); - return () => listenersRef.current.delete(listener); - }, - [] - ); - - React.useEffect(() => { - for (const listener of listenersRef.current) { - listener(options); - } - }, [options]); - - const value = React.useMemo( - () => ({ initialState: initialState.current as O, subscribe }), - [initialState.current, subscribe] - ); - - element = React.createElement(context.Provider, { value }, element); - } - } - + const element = useCreateElement(as, allProps as typeof props); if (wrapElement) { return wrapElement(element); } @@ -142,10 +102,6 @@ export function createComponent({ Comp = forwardRef(Comp); - if (context && !isContextProvider) { - Comp = withStateContextSubscriber(Comp, context); - } - if (shouldMemo) { Comp = memo(Comp, propsAreEqual && normalizePropsAreEqual(propsAreEqual)); } diff --git a/packages/reakit-system/src/useStateContext.ts b/packages/reakit-system/src/useStateContext.ts new file mode 100644 index 00000000..7d1cc416 --- /dev/null +++ b/packages/reakit-system/src/useStateContext.ts @@ -0,0 +1,6 @@ +import React from "react"; +import { StateContext, StateContextValue } from "./withStateContextSubscriber"; + +export function useStateContext(ctx: StateContext): StateContextValue { + return React.useContext(ctx); +} diff --git a/packages/reakit-system/src/withStateContextSubscriber.tsx b/packages/reakit-system/src/withStateContextSubscriber.tsx index b744d762..e7a74f43 100644 --- a/packages/reakit-system/src/withStateContextSubscriber.tsx +++ b/packages/reakit-system/src/withStateContextSubscriber.tsx @@ -1,5 +1,7 @@ import React from "react"; import { unstable_useId as useId } from "reakit/Id"; +import { createHook } from "./createHook"; +import { useStateContext } from "./useStateContext"; export type StateContext = React.Context>; export type StateContextValue = { @@ -48,3 +50,85 @@ export function withStateContextSubscriber< return (inner as unknown) as T; } + +export const useStateContextProvider = (context: StateContext) => + createHook<{}, {}>({ + name: "StateContextProvider", + useProps: (options, htmlProps) => { + const wrapElement = (element) => { + const initialState = React.useRef(); + if (!initialState.current) { + initialState.current = options; + } + + if (htmlProps.wrapElement) { + element = htmlProps.wrapElement(element); + } + const listenersRef = React.useRef(new Set>()); + + const subscribe: StateContextSubscribe = React.useCallback( + (listener: StateContextListener) => { + listenersRef.current.add(listener); + return () => listenersRef.current.delete(listener); + }, + [] + ); + + React.useEffect(() => { + for (const listener of listenersRef.current) { + listener(options); + } + }, [options]); + + const value = React.useMemo( + () => ({ initialState: initialState.current as O, subscribe }), + [initialState.current, subscribe] + ); + element = React.createElement(context.Provider, { value }, element); + return element; + }; + + return { + ...htmlProps, + wrapElement, + }; + }, + }); + +export const useStateContextSubscribe = (ctx: StateContext) => + createHook<{}, React.HTMLAttributes>({ + name: "StateContextSubscribe", + + useOptions(options, htmlProps) { + const context = useStateContext(ctx); + + let subscribe: StateContextSubscribe | null = null; + let initialState: O = {} as O; + if (context) { + const { subscribe: _subscribe, initialState: _initialState } = context; + subscribe = _subscribe; + initialState = _initialState; + } + + const { id } = useId({ ...initialState, ...options, ...htmlProps }); + const [state, setState] = React.useState(initialState); + + React.useEffect( + () => + subscribe + ? subscribe((nextState: any) => { + if ( + state.currentId === null || + id === state.currentId || + id === nextState.currentId + ) { + setState(nextState); + } + }) + : undefined, + [subscribe, state?.currentId, id, htmlProps.id] + ); + + return { ...options, ...state }; + }, + }); diff --git a/packages/reakit/src/Combobox/ComboboxOption.ts b/packages/reakit/src/Combobox/ComboboxOption.ts index 81fae9d3..0415eaed 100644 --- a/packages/reakit/src/Combobox/ComboboxOption.ts +++ b/packages/reakit/src/Combobox/ComboboxOption.ts @@ -1,5 +1,6 @@ import { createComponent } from "reakit-system/createComponent"; import { createHook } from "reakit-system/createHook"; +import { useStateContextSubscribe } from "reakit-system/withStateContextSubscriber"; import { CompositeItemOptions, CompositeItemHTMLProps, @@ -18,7 +19,11 @@ export const unstable_useComboboxOption = createHook< unstable_ComboboxOptionHTMLProps >({ name: "ComboboxOption", - compose: [useComboboxItem, useCompositeItem], + compose: [ + useComboboxItem, + useCompositeItem, + useStateContextSubscribe(StateContext), + ], keys: COMBOBOX_OPTION_KEYS, useProps(_, htmlProps) { @@ -30,7 +35,6 @@ export const unstable_ComboboxOption = createComponent({ as: "div", memo: true, useHook: unstable_useComboboxOption, - context: StateContext, }); export type unstable_ComboboxOptionOptions = CompositeItemOptions & diff --git a/packages/reakit/src/Combobox/ComboboxPopover.ts b/packages/reakit/src/Combobox/ComboboxPopover.ts index 6b8a4be2..34d2610e 100644 --- a/packages/reakit/src/Combobox/ComboboxPopover.ts +++ b/packages/reakit/src/Combobox/ComboboxPopover.ts @@ -3,6 +3,7 @@ import { createHook } from "reakit-system/createHook"; import { useWarning } from "reakit-warning"; import { useCreateElement } from "reakit-system/useCreateElement"; import { createStateContext } from "reakit-system/createStateContext"; +import { useStateContextProvider } from "reakit-system/withStateContextSubscriber"; import { PopoverOptions, PopoverHTMLProps, @@ -16,12 +17,14 @@ import { } from "./ComboboxList"; import { ComboboxPopoverStateReturn } from "./__utils/ComboboxPopoverState"; +export const StateContext = createStateContext(); + export const unstable_useComboboxPopover = createHook< unstable_ComboboxPopoverOptions, unstable_ComboboxPopoverHTMLProps >({ name: "ComboboxPopover", - compose: [useComboboxList, usePopover], + compose: [useComboboxList, usePopover, useStateContextProvider(StateContext)], keys: COMBOBOX_POPOVER_KEYS, useOptions(options) { @@ -36,6 +39,7 @@ export const unstable_useComboboxPopover = createHook< useComposeProps(options, { tabIndex, ...htmlProps }) { htmlProps = useComboboxList(options, htmlProps, true); htmlProps = usePopover(options, htmlProps, true); + htmlProps = useStateContextProvider(StateContext)(options, htmlProps, true); return { ...htmlProps, tabIndex: tabIndex ?? undefined, @@ -43,13 +47,9 @@ export const unstable_useComboboxPopover = createHook< }, }); -export const StateContext = createStateContext(); - export const unstable_ComboboxPopover = createComponent({ as: "div", useHook: unstable_useComboboxPopover, - context: StateContext, - isContextProvider: true, useCreateElement: (type, props, children) => { useWarning( !props["aria-label"] && !props["aria-labelledby"], From c800ef7d78c6c48fe639f045b3c19b532c3e325d Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 12:48:56 +0100 Subject: [PATCH 10/28] Cleanup --- packages/reakit-system/src/createComponent.ts | 3 -- .../src/withStateContextSubscriber.tsx | 40 ------------------- 2 files changed, 43 deletions(-) diff --git a/packages/reakit-system/src/createComponent.ts b/packages/reakit-system/src/createComponent.ts index 7d1e80bf..1fee9226 100644 --- a/packages/reakit-system/src/createComponent.ts +++ b/packages/reakit-system/src/createComponent.ts @@ -56,8 +56,6 @@ export function createComponent({ as: type, useHook, memo: shouldMemo, - context, - isContextProvider = false, propsAreEqual = useHook?.unstable_propsAreEqual, keys = useHook?.__keys || [], useCreateElement = defaultUseCreateElement, @@ -86,7 +84,6 @@ export function createComponent({ : asOptions ? { ...elementProps, ...asOptions } : elementProps; - const element = useCreateElement(as, allProps as typeof props); if (wrapElement) { return wrapElement(element); diff --git a/packages/reakit-system/src/withStateContextSubscriber.tsx b/packages/reakit-system/src/withStateContextSubscriber.tsx index e7a74f43..4eee5bb9 100644 --- a/packages/reakit-system/src/withStateContextSubscriber.tsx +++ b/packages/reakit-system/src/withStateContextSubscriber.tsx @@ -11,46 +11,6 @@ export type StateContextValue = { export type StateContextListener = (options: O) => void; export type StateContextSubscribe = (callback: (options: O) => any) => any; -export function withStateContextSubscriber< - T extends React.ComponentType, - O extends React.HTMLAttributes ->(Comp: T, ctx: StateContext): T { - const inner = (props: O) => { - const context = React.useContext(ctx); - - let subscribe: StateContextSubscribe | null = null; - let initialState: O = {} as O; - if (context) { - const { subscribe: _subscribe, initialState: _initialState } = context; - subscribe = _subscribe; - initialState = _initialState; - } - - const { id } = useId({ ...initialState, ...props }); - const [state, setState] = React.useState(initialState); - - React.useEffect( - () => - subscribe - ? subscribe((nextState: any) => { - if ( - state.currentId === null || - id === state.currentId || - id === nextState.currentId - ) { - setState(nextState); - } - }) - : undefined, - [subscribe, state?.currentId, id, props.id] - ); - - return ; - }; - - return (inner as unknown) as T; -} - export const useStateContextProvider = (context: StateContext) => createHook<{}, {}>({ name: "StateContextProvider", From e5e1055c224d1c651c31cf558242e4184f6cb0e7 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 12:53:38 +0100 Subject: [PATCH 11/28] Rename --- packages/reakit-system/src/useStateContext.ts | 9 +- .../src/useStateContextConsumer.tsx | 46 +++++++++ .../src/useStateContextProvider.tsx | 51 ++++++++++ .../src/withStateContextSubscriber.tsx | 94 ------------------- .../reakit/src/Combobox/ComboboxOption.ts | 4 +- .../reakit/src/Combobox/ComboboxPopover.ts | 2 +- 6 files changed, 108 insertions(+), 98 deletions(-) create mode 100644 packages/reakit-system/src/useStateContextConsumer.tsx create mode 100644 packages/reakit-system/src/useStateContextProvider.tsx delete mode 100644 packages/reakit-system/src/withStateContextSubscriber.tsx diff --git a/packages/reakit-system/src/useStateContext.ts b/packages/reakit-system/src/useStateContext.ts index 7d1cc416..b1a64573 100644 --- a/packages/reakit-system/src/useStateContext.ts +++ b/packages/reakit-system/src/useStateContext.ts @@ -1,5 +1,12 @@ import React from "react"; -import { StateContext, StateContextValue } from "./withStateContextSubscriber"; + +export type StateContext = React.Context>; +export type StateContextValue = { + initialState: O; + subscribe: StateContextSubscribe; +}; +export type StateContextListener = (options: O) => void; +export type StateContextSubscribe = (callback: (options: O) => any) => any; export function useStateContext(ctx: StateContext): StateContextValue { return React.useContext(ctx); diff --git a/packages/reakit-system/src/useStateContextConsumer.tsx b/packages/reakit-system/src/useStateContextConsumer.tsx new file mode 100644 index 00000000..618ac76a --- /dev/null +++ b/packages/reakit-system/src/useStateContextConsumer.tsx @@ -0,0 +1,46 @@ +import React from "react"; +import { unstable_useId as useId } from "reakit/Id"; +import { createHook } from "./createHook"; +import { + StateContext, + StateContextSubscribe, + useStateContext, +} from "./useStateContext"; + +export const useStateContextConsumer = (ctx: StateContext) => + createHook<{}, React.HTMLAttributes>({ + name: "StateContextSubscribe", + + useOptions(options, htmlProps) { + const context = useStateContext(ctx); + + let subscribe: StateContextSubscribe | null = null; + let initialState: O = {} as O; + if (context) { + const { subscribe: _subscribe, initialState: _initialState } = context; + subscribe = _subscribe; + initialState = _initialState; + } + + const { id } = useId({ ...initialState, ...options, ...htmlProps }); + const [state, setState] = React.useState(initialState); + + React.useEffect( + () => + subscribe + ? subscribe((nextState: any) => { + if ( + state.currentId === null || + id === state.currentId || + id === nextState.currentId + ) { + setState(nextState); + } + }) + : undefined, + [subscribe, state?.currentId, id, htmlProps.id] + ); + + return { ...options, ...state }; + }, + }); diff --git a/packages/reakit-system/src/useStateContextProvider.tsx b/packages/reakit-system/src/useStateContextProvider.tsx new file mode 100644 index 00000000..ca1c087c --- /dev/null +++ b/packages/reakit-system/src/useStateContextProvider.tsx @@ -0,0 +1,51 @@ +import React from "react"; +import { createHook } from "./createHook"; +import { + StateContext, + StateContextListener, + StateContextSubscribe, +} from "./useStateContext"; + +export const useStateContextProvider = (context: StateContext) => + createHook<{}, {}>({ + name: "StateContextProvider", + useProps: (options, htmlProps) => { + const wrapElement = (element) => { + const initialState = React.useRef(); + if (!initialState.current) { + initialState.current = options; + } + + if (htmlProps.wrapElement) { + element = htmlProps.wrapElement(element); + } + const listenersRef = React.useRef(new Set>()); + + const subscribe: StateContextSubscribe = React.useCallback( + (listener: StateContextListener) => { + listenersRef.current.add(listener); + return () => listenersRef.current.delete(listener); + }, + [] + ); + + React.useEffect(() => { + for (const listener of listenersRef.current) { + listener(options); + } + }, [options]); + + const value = React.useMemo( + () => ({ initialState: initialState.current as O, subscribe }), + [initialState.current, subscribe] + ); + element = React.createElement(context.Provider, { value }, element); + return element; + }; + + return { + ...htmlProps, + wrapElement, + }; + }, + }); diff --git a/packages/reakit-system/src/withStateContextSubscriber.tsx b/packages/reakit-system/src/withStateContextSubscriber.tsx deleted file mode 100644 index 4eee5bb9..00000000 --- a/packages/reakit-system/src/withStateContextSubscriber.tsx +++ /dev/null @@ -1,94 +0,0 @@ -import React from "react"; -import { unstable_useId as useId } from "reakit/Id"; -import { createHook } from "./createHook"; -import { useStateContext } from "./useStateContext"; - -export type StateContext = React.Context>; -export type StateContextValue = { - initialState: O; - subscribe: StateContextSubscribe; -}; -export type StateContextListener = (options: O) => void; -export type StateContextSubscribe = (callback: (options: O) => any) => any; - -export const useStateContextProvider = (context: StateContext) => - createHook<{}, {}>({ - name: "StateContextProvider", - useProps: (options, htmlProps) => { - const wrapElement = (element) => { - const initialState = React.useRef(); - if (!initialState.current) { - initialState.current = options; - } - - if (htmlProps.wrapElement) { - element = htmlProps.wrapElement(element); - } - const listenersRef = React.useRef(new Set>()); - - const subscribe: StateContextSubscribe = React.useCallback( - (listener: StateContextListener) => { - listenersRef.current.add(listener); - return () => listenersRef.current.delete(listener); - }, - [] - ); - - React.useEffect(() => { - for (const listener of listenersRef.current) { - listener(options); - } - }, [options]); - - const value = React.useMemo( - () => ({ initialState: initialState.current as O, subscribe }), - [initialState.current, subscribe] - ); - element = React.createElement(context.Provider, { value }, element); - return element; - }; - - return { - ...htmlProps, - wrapElement, - }; - }, - }); - -export const useStateContextSubscribe = (ctx: StateContext) => - createHook<{}, React.HTMLAttributes>({ - name: "StateContextSubscribe", - - useOptions(options, htmlProps) { - const context = useStateContext(ctx); - - let subscribe: StateContextSubscribe | null = null; - let initialState: O = {} as O; - if (context) { - const { subscribe: _subscribe, initialState: _initialState } = context; - subscribe = _subscribe; - initialState = _initialState; - } - - const { id } = useId({ ...initialState, ...options, ...htmlProps }); - const [state, setState] = React.useState(initialState); - - React.useEffect( - () => - subscribe - ? subscribe((nextState: any) => { - if ( - state.currentId === null || - id === state.currentId || - id === nextState.currentId - ) { - setState(nextState); - } - }) - : undefined, - [subscribe, state?.currentId, id, htmlProps.id] - ); - - return { ...options, ...state }; - }, - }); diff --git a/packages/reakit/src/Combobox/ComboboxOption.ts b/packages/reakit/src/Combobox/ComboboxOption.ts index 0415eaed..bd085faf 100644 --- a/packages/reakit/src/Combobox/ComboboxOption.ts +++ b/packages/reakit/src/Combobox/ComboboxOption.ts @@ -1,6 +1,6 @@ import { createComponent } from "reakit-system/createComponent"; import { createHook } from "reakit-system/createHook"; -import { useStateContextSubscribe } from "reakit-system/withStateContextSubscriber"; +import { useStateContextConsumer } from "reakit-system/useStateContextConsumer"; import { CompositeItemOptions, CompositeItemHTMLProps, @@ -22,7 +22,7 @@ export const unstable_useComboboxOption = createHook< compose: [ useComboboxItem, useCompositeItem, - useStateContextSubscribe(StateContext), + useStateContextConsumer(StateContext), ], keys: COMBOBOX_OPTION_KEYS, diff --git a/packages/reakit/src/Combobox/ComboboxPopover.ts b/packages/reakit/src/Combobox/ComboboxPopover.ts index 34d2610e..7ea20a3a 100644 --- a/packages/reakit/src/Combobox/ComboboxPopover.ts +++ b/packages/reakit/src/Combobox/ComboboxPopover.ts @@ -3,7 +3,7 @@ import { createHook } from "reakit-system/createHook"; import { useWarning } from "reakit-warning"; import { useCreateElement } from "reakit-system/useCreateElement"; import { createStateContext } from "reakit-system/createStateContext"; -import { useStateContextProvider } from "reakit-system/withStateContextSubscriber"; +import { useStateContextProvider } from "reakit-system/useStateContextProvider"; import { PopoverOptions, PopoverHTMLProps, From 6292bf9307beb5fe763f537f7314876654f30164 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 12:53:47 +0100 Subject: [PATCH 12/28] Cleanup story --- .../ComboboxNonContextVsContext/index.tsx | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx index 420e9893..ef73fa3b 100644 --- a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx +++ b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx @@ -7,28 +7,6 @@ import { import "./style.css"; import * as React from "react"; -function ComboboxOriginal() { - const items = Array.from({ length: 250 }).map((_, i) => `Item ${i}`); - - const combobox = useComboboxState({ - values: items, - gutter: 8, - limit: false, - autoSelect: true, - }); - - return ( - <> - - - {combobox.matches.map((value) => ( - - ))} - - - ); -} - function ComboboxContext() { const items = Array.from({ length: 250 }).map((_, i) => `Item ${i}`); @@ -54,16 +32,6 @@ function ComboboxContext() { export default function ComboBoxNonContextVsContext() { return ( <> -
-

- Original Combobox, clicking on it may take some time. - Use arrows (hold down your down-arrow key) to navigate between - elements. You will notice how slow it is. -

- - -
-

New Combobox using Context, clicking on it takes less From 2ae46a7a4c0c1cfd8ba7542bab57cdb5602a9bf0 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 12:55:05 +0100 Subject: [PATCH 13/28] Restore story --- .../ComboboxNonContextVsContext/index.tsx | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx index ef73fa3b..420e9893 100644 --- a/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx +++ b/packages/reakit/src/Combobox/__examples__/ComboboxNonContextVsContext/index.tsx @@ -7,6 +7,28 @@ import { import "./style.css"; import * as React from "react"; +function ComboboxOriginal() { + const items = Array.from({ length: 250 }).map((_, i) => `Item ${i}`); + + const combobox = useComboboxState({ + values: items, + gutter: 8, + limit: false, + autoSelect: true, + }); + + return ( + <> + + + {combobox.matches.map((value) => ( + + ))} + + + ); +} + function ComboboxContext() { const items = Array.from({ length: 250 }).map((_, i) => `Item ${i}`); @@ -32,6 +54,16 @@ function ComboboxContext() { export default function ComboBoxNonContextVsContext() { return ( <> +

+

+ Original Combobox, clicking on it may take some time. + Use arrows (hold down your down-arrow key) to navigate between + elements. You will notice how slow it is. +

+ + +
+

New Combobox using Context, clicking on it takes less From abf42d6b3314484851f21a8a84e81e2281aa8b7d Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 13:01:58 +0100 Subject: [PATCH 14/28] Fix error --- packages/reakit-system/src/createStateContext.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reakit-system/src/createStateContext.ts b/packages/reakit-system/src/createStateContext.ts index 27143e10..61d50359 100644 --- a/packages/reakit-system/src/createStateContext.ts +++ b/packages/reakit-system/src/createStateContext.ts @@ -1,5 +1,5 @@ import React from "react"; -import { StateContext } from "./withStateContextSubscriber"; +import { StateContext } from "./useStateContext"; export function createStateContext(): StateContext { return (React.createContext({ From bc67d4d9abb7c952bb0366f5652b5d1fdaa85b63 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 13:11:45 +0100 Subject: [PATCH 15/28] Move to own folder --- packages/reakit/src/Combobox/ComboboxOption.ts | 2 +- packages/reakit/src/Combobox/ComboboxPopover.ts | 4 ++-- .../src/StateContext}/createStateContext.ts | 0 packages/reakit/src/StateContext/index.ts | 4 ++++ .../src/StateContext}/useStateContext.ts | 0 .../src/StateContext}/useStateContextConsumer.tsx | 2 +- .../src/StateContext}/useStateContextProvider.tsx | 15 ++++++++++++--- 7 files changed, 20 insertions(+), 7 deletions(-) rename packages/{reakit-system/src => reakit/src/StateContext}/createStateContext.ts (100%) create mode 100644 packages/reakit/src/StateContext/index.ts rename packages/{reakit-system/src => reakit/src/StateContext}/useStateContext.ts (100%) rename packages/{reakit-system/src => reakit/src/StateContext}/useStateContextConsumer.tsx (96%) rename packages/{reakit-system/src => reakit/src/StateContext}/useStateContextProvider.tsx (75%) diff --git a/packages/reakit/src/Combobox/ComboboxOption.ts b/packages/reakit/src/Combobox/ComboboxOption.ts index bd085faf..c946d9ac 100644 --- a/packages/reakit/src/Combobox/ComboboxOption.ts +++ b/packages/reakit/src/Combobox/ComboboxOption.ts @@ -1,6 +1,6 @@ import { createComponent } from "reakit-system/createComponent"; import { createHook } from "reakit-system/createHook"; -import { useStateContextConsumer } from "reakit-system/useStateContextConsumer"; +import { useStateContextConsumer } from "reakit/StateContext/useStateContextConsumer"; import { CompositeItemOptions, CompositeItemHTMLProps, diff --git a/packages/reakit/src/Combobox/ComboboxPopover.ts b/packages/reakit/src/Combobox/ComboboxPopover.ts index 7ea20a3a..0e36ba31 100644 --- a/packages/reakit/src/Combobox/ComboboxPopover.ts +++ b/packages/reakit/src/Combobox/ComboboxPopover.ts @@ -2,8 +2,8 @@ import { createComponent } from "reakit-system/createComponent"; import { createHook } from "reakit-system/createHook"; import { useWarning } from "reakit-warning"; import { useCreateElement } from "reakit-system/useCreateElement"; -import { createStateContext } from "reakit-system/createStateContext"; -import { useStateContextProvider } from "reakit-system/useStateContextProvider"; +import { useStateContextProvider } from "reakit/StateContext/useStateContextProvider"; +import { createStateContext } from "reakit/StateContext/createStateContext"; import { PopoverOptions, PopoverHTMLProps, diff --git a/packages/reakit-system/src/createStateContext.ts b/packages/reakit/src/StateContext/createStateContext.ts similarity index 100% rename from packages/reakit-system/src/createStateContext.ts rename to packages/reakit/src/StateContext/createStateContext.ts diff --git a/packages/reakit/src/StateContext/index.ts b/packages/reakit/src/StateContext/index.ts new file mode 100644 index 00000000..7d463882 --- /dev/null +++ b/packages/reakit/src/StateContext/index.ts @@ -0,0 +1,4 @@ +export * from "./createStateContext"; +export * from "./useStateContext"; +export * from "./useStateContextConsumer"; +export * from "./useStateContextProvider"; diff --git a/packages/reakit-system/src/useStateContext.ts b/packages/reakit/src/StateContext/useStateContext.ts similarity index 100% rename from packages/reakit-system/src/useStateContext.ts rename to packages/reakit/src/StateContext/useStateContext.ts diff --git a/packages/reakit-system/src/useStateContextConsumer.tsx b/packages/reakit/src/StateContext/useStateContextConsumer.tsx similarity index 96% rename from packages/reakit-system/src/useStateContextConsumer.tsx rename to packages/reakit/src/StateContext/useStateContextConsumer.tsx index 618ac76a..1171c8db 100644 --- a/packages/reakit-system/src/useStateContextConsumer.tsx +++ b/packages/reakit/src/StateContext/useStateContextConsumer.tsx @@ -1,6 +1,6 @@ import React from "react"; import { unstable_useId as useId } from "reakit/Id"; -import { createHook } from "./createHook"; +import { createHook } from "reakit-system/createHook"; import { StateContext, StateContextSubscribe, diff --git a/packages/reakit-system/src/useStateContextProvider.tsx b/packages/reakit/src/StateContext/useStateContextProvider.tsx similarity index 75% rename from packages/reakit-system/src/useStateContextProvider.tsx rename to packages/reakit/src/StateContext/useStateContextProvider.tsx index ca1c087c..e6e4a0de 100644 --- a/packages/reakit-system/src/useStateContextProvider.tsx +++ b/packages/reakit/src/StateContext/useStateContextProvider.tsx @@ -1,16 +1,25 @@ import React from "react"; -import { createHook } from "./createHook"; +import { createHook } from "reakit-system/createHook"; import { StateContext, StateContextListener, StateContextSubscribe, } from "./useStateContext"; +export type useStateContextProviderHTMLProps = React.HTMLAttributes & + React.RefAttributes & { + /** + * Function returned by the hook to wrap the element to which html props + * will be passed. + */ + wrapElement?: (element: React.ReactNode) => React.ReactNode; + }; + export const useStateContextProvider = (context: StateContext) => - createHook<{}, {}>({ + createHook({ name: "StateContextProvider", useProps: (options, htmlProps) => { - const wrapElement = (element) => { + const wrapElement = (element: React.ReactNode) => { const initialState = React.useRef(); if (!initialState.current) { initialState.current = options; From b908a81e86cb885a24b4f40c14a2e1f6b72c69d2 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 13:13:38 +0100 Subject: [PATCH 16/28] Typing --- packages/reakit/src/StateContext/useStateContextConsumer.tsx | 2 +- packages/reakit/src/StateContext/useStateContextProvider.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/reakit/src/StateContext/useStateContextConsumer.tsx b/packages/reakit/src/StateContext/useStateContextConsumer.tsx index 1171c8db..cf16838d 100644 --- a/packages/reakit/src/StateContext/useStateContextConsumer.tsx +++ b/packages/reakit/src/StateContext/useStateContextConsumer.tsx @@ -8,7 +8,7 @@ import { } from "./useStateContext"; export const useStateContextConsumer = (ctx: StateContext) => - createHook<{}, React.HTMLAttributes>({ + createHook>({ name: "StateContextSubscribe", useOptions(options, htmlProps) { diff --git a/packages/reakit/src/StateContext/useStateContextProvider.tsx b/packages/reakit/src/StateContext/useStateContextProvider.tsx index e6e4a0de..8d9cea36 100644 --- a/packages/reakit/src/StateContext/useStateContextProvider.tsx +++ b/packages/reakit/src/StateContext/useStateContextProvider.tsx @@ -6,7 +6,7 @@ import { StateContextSubscribe, } from "./useStateContext"; -export type useStateContextProviderHTMLProps = React.HTMLAttributes & +export type UseStateContextProviderHTMLProps = React.HTMLAttributes & React.RefAttributes & { /** * Function returned by the hook to wrap the element to which html props @@ -16,7 +16,7 @@ export type useStateContextProviderHTMLProps = React.HTMLAttributes & }; export const useStateContextProvider = (context: StateContext) => - createHook({ + createHook({ name: "StateContextProvider", useProps: (options, htmlProps) => { const wrapElement = (element: React.ReactNode) => { From 05c00003b4902814dc83cee75764e8410b7f2e12 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 13:16:32 +0100 Subject: [PATCH 17/28] Update name --- packages/reakit/src/StateContext/useStateContextConsumer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reakit/src/StateContext/useStateContextConsumer.tsx b/packages/reakit/src/StateContext/useStateContextConsumer.tsx index cf16838d..375073af 100644 --- a/packages/reakit/src/StateContext/useStateContextConsumer.tsx +++ b/packages/reakit/src/StateContext/useStateContextConsumer.tsx @@ -9,7 +9,7 @@ import { export const useStateContextConsumer = (ctx: StateContext) => createHook>({ - name: "StateContextSubscribe", + name: "StateContextConsumer", useOptions(options, htmlProps) { const context = useStateContext(ctx); From 2eecaa7dcd95b2a848845c8743fb5ffd3680592a Mon Sep 17 00:00:00 2001 From: David Szabo Date: Fri, 27 Nov 2020 13:35:32 +0100 Subject: [PATCH 18/28] Add shouldUpdate and updateDeps --- .../reakit/src/Combobox/ComboboxOption.ts | 9 ++++++- .../reakit/src/Combobox/ComboboxPopover.ts | 5 +++- .../StateContext/useStateContextConsumer.tsx | 24 +++++++++++-------- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/packages/reakit/src/Combobox/ComboboxOption.ts b/packages/reakit/src/Combobox/ComboboxOption.ts index c946d9ac..a394ab19 100644 --- a/packages/reakit/src/Combobox/ComboboxOption.ts +++ b/packages/reakit/src/Combobox/ComboboxOption.ts @@ -22,7 +22,14 @@ export const unstable_useComboboxOption = createHook< compose: [ useComboboxItem, useCompositeItem, - useStateContextConsumer(StateContext), + useStateContextConsumer({ + context: StateContext, + shouldUpdate: (id, state, nextState) => + state.currentId === null || + id === state.currentId || + id === nextState.currentId, + updateDependencies: (state) => [state?.currentId], + }), ], keys: COMBOBOX_OPTION_KEYS, diff --git a/packages/reakit/src/Combobox/ComboboxPopover.ts b/packages/reakit/src/Combobox/ComboboxPopover.ts index 0e36ba31..74ab0a92 100644 --- a/packages/reakit/src/Combobox/ComboboxPopover.ts +++ b/packages/reakit/src/Combobox/ComboboxPopover.ts @@ -16,8 +16,11 @@ import { unstable_useComboboxList as useComboboxList, } from "./ComboboxList"; import { ComboboxPopoverStateReturn } from "./__utils/ComboboxPopoverState"; +import { unstable_ComboboxState } from "./ComboboxState"; -export const StateContext = createStateContext(); +export const StateContext = createStateContext< + Partial +>(); export const unstable_useComboboxPopover = createHook< unstable_ComboboxPopoverOptions, diff --git a/packages/reakit/src/StateContext/useStateContextConsumer.tsx b/packages/reakit/src/StateContext/useStateContextConsumer.tsx index 375073af..0cdbde38 100644 --- a/packages/reakit/src/StateContext/useStateContextConsumer.tsx +++ b/packages/reakit/src/StateContext/useStateContextConsumer.tsx @@ -7,17 +7,25 @@ import { useStateContext, } from "./useStateContext"; -export const useStateContextConsumer = (ctx: StateContext) => +export const useStateContextConsumer = ({ + context, + shouldUpdate, + updateDependencies = () => [], +}: { + context: StateContext; + shouldUpdate: (id: string | undefined, state: O, nextState: O) => boolean; + updateDependencies: (state: O) => any[]; +}) => createHook>({ name: "StateContextConsumer", useOptions(options, htmlProps) { - const context = useStateContext(ctx); + const ctx = useStateContext(context); let subscribe: StateContextSubscribe | null = null; let initialState: O = {} as O; - if (context) { - const { subscribe: _subscribe, initialState: _initialState } = context; + if (ctx) { + const { subscribe: _subscribe, initialState: _initialState } = ctx; subscribe = _subscribe; initialState = _initialState; } @@ -29,16 +37,12 @@ export const useStateContextConsumer = (ctx: StateContext) => () => subscribe ? subscribe((nextState: any) => { - if ( - state.currentId === null || - id === state.currentId || - id === nextState.currentId - ) { + if (shouldUpdate(id, state, nextState)) { setState(nextState); } }) : undefined, - [subscribe, state?.currentId, id, htmlProps.id] + [subscribe, id, htmlProps.id, ...updateDependencies(state)] ); return { ...options, ...state }; From 03b684d0ebb82dd870f474abf3efab7962a3962b Mon Sep 17 00:00:00 2001 From: David Szabo Date: Tue, 1 Dec 2020 09:38:44 +0100 Subject: [PATCH 19/28] Remove old export --- packages/reakit-system/src/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/reakit-system/src/index.ts b/packages/reakit-system/src/index.ts index 5a36f9af..2370206d 100644 --- a/packages/reakit-system/src/index.ts +++ b/packages/reakit-system/src/index.ts @@ -1,6 +1,5 @@ export * from "./createComponent"; export * from "./createHook"; -export * from "./createStateContext"; export * from "./mergeSystem"; export * from "./SystemContext"; export * from "./SystemProvider"; From ac22b28470d07a6f3791a87d11bb65809a0cd8b2 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Tue, 9 Feb 2021 13:45:36 +0100 Subject: [PATCH 20/28] Fix --- packages/reakit/src/Combobox/ComboboxOption.ts | 6 +++--- packages/reakit/src/Combobox/ComboboxPopover.ts | 11 +++++++---- .../src/StateContext/useStateContextConsumer.tsx | 3 +-- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/reakit/src/Combobox/ComboboxOption.ts b/packages/reakit/src/Combobox/ComboboxOption.ts index a394ab19..7684f4eb 100644 --- a/packages/reakit/src/Combobox/ComboboxOption.ts +++ b/packages/reakit/src/Combobox/ComboboxOption.ts @@ -1,6 +1,6 @@ import { createComponent } from "reakit-system/createComponent"; import { createHook } from "reakit-system/createHook"; -import { useStateContextConsumer } from "reakit/StateContext/useStateContextConsumer"; +import { useStateContextConsumer } from "reakit/StateContext"; import { CompositeItemOptions, CompositeItemHTMLProps, @@ -20,8 +20,6 @@ export const unstable_useComboboxOption = createHook< >({ name: "ComboboxOption", compose: [ - useComboboxItem, - useCompositeItem, useStateContextConsumer({ context: StateContext, shouldUpdate: (id, state, nextState) => @@ -30,6 +28,8 @@ export const unstable_useComboboxOption = createHook< id === nextState.currentId, updateDependencies: (state) => [state?.currentId], }), + useComboboxItem, + useCompositeItem, ], keys: COMBOBOX_OPTION_KEYS, diff --git a/packages/reakit/src/Combobox/ComboboxPopover.ts b/packages/reakit/src/Combobox/ComboboxPopover.ts index 74ab0a92..8579a73e 100644 --- a/packages/reakit/src/Combobox/ComboboxPopover.ts +++ b/packages/reakit/src/Combobox/ComboboxPopover.ts @@ -2,8 +2,10 @@ import { createComponent } from "reakit-system/createComponent"; import { createHook } from "reakit-system/createHook"; import { useWarning } from "reakit-warning"; import { useCreateElement } from "reakit-system/useCreateElement"; -import { useStateContextProvider } from "reakit/StateContext/useStateContextProvider"; -import { createStateContext } from "reakit/StateContext/createStateContext"; +import { + createStateContext, + useStateContextProvider, +} from "reakit/StateContext"; import { PopoverOptions, PopoverHTMLProps, @@ -27,7 +29,7 @@ export const unstable_useComboboxPopover = createHook< unstable_ComboboxPopoverHTMLProps >({ name: "ComboboxPopover", - compose: [useComboboxList, usePopover, useStateContextProvider(StateContext)], + compose: [useStateContextProvider(StateContext), useComboboxList, usePopover], keys: COMBOBOX_POPOVER_KEYS, useOptions(options) { @@ -40,9 +42,10 @@ export const unstable_useComboboxPopover = createHook< }, useComposeProps(options, { tabIndex, ...htmlProps }) { + htmlProps = useStateContextProvider(StateContext)(options, htmlProps, true); htmlProps = useComboboxList(options, htmlProps, true); htmlProps = usePopover(options, htmlProps, true); - htmlProps = useStateContextProvider(StateContext)(options, htmlProps, true); + return { ...htmlProps, tabIndex: tabIndex ?? undefined, diff --git a/packages/reakit/src/StateContext/useStateContextConsumer.tsx b/packages/reakit/src/StateContext/useStateContextConsumer.tsx index 0cdbde38..3cd809cd 100644 --- a/packages/reakit/src/StateContext/useStateContextConsumer.tsx +++ b/packages/reakit/src/StateContext/useStateContextConsumer.tsx @@ -44,7 +44,6 @@ export const useStateContextConsumer = ({ : undefined, [subscribe, id, htmlProps.id, ...updateDependencies(state)] ); - - return { ...options, ...state }; + return { ...state, ...options, id }; }, }); From ad906004445ea90171c5316ca9d63d7a07569421 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Tue, 9 Feb 2021 14:25:24 +0100 Subject: [PATCH 21/28] Update --- packages/reakit/src/Combobox/ComboboxOption.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/reakit/src/Combobox/ComboboxOption.ts b/packages/reakit/src/Combobox/ComboboxOption.ts index 7684f4eb..3cd852da 100644 --- a/packages/reakit/src/Combobox/ComboboxOption.ts +++ b/packages/reakit/src/Combobox/ComboboxOption.ts @@ -22,10 +22,14 @@ export const unstable_useComboboxOption = createHook< compose: [ useStateContextConsumer({ context: StateContext, - shouldUpdate: (id, state, nextState) => - state.currentId === null || - id === state.currentId || - id === nextState.currentId, + shouldUpdate: (id, state, nextState) => { + return ( + state.currentId === null || + nextState.currentId === null || + id === state.currentId || + id === nextState.currentId + ); + }, updateDependencies: (state) => [state?.currentId], }), useComboboxItem, From 50eaa297987ff365d39143467511db07360df255 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Tue, 9 Feb 2021 14:25:35 +0100 Subject: [PATCH 22/28] Add story --- .../__tests__/index-test.tsx | 236 ++++++++++++++++++ .../AccessibleComboboxContext/index.tsx | 23 ++ .../AccessibleComboboxContext/style.css | 72 ++++++ .../src/Combobox/__examples__/index.story.ts | 1 + 4 files changed, 332 insertions(+) create mode 100644 packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/__tests__/index-test.tsx create mode 100644 packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/index.tsx create mode 100644 packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/style.css diff --git a/packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/__tests__/index-test.tsx b/packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/__tests__/index-test.tsx new file mode 100644 index 00000000..c2c750c5 --- /dev/null +++ b/packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/__tests__/index-test.tsx @@ -0,0 +1,236 @@ +import * as React from "react"; +import { render, press, click, type, screen } from "reakit-test-utils"; +import AccessibleCombobox from ".."; + +test("open combobox popover on click", () => { + render(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + click(screen.getByLabelText("Fruit")); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + expect(screen.getByText("Apple")).not.toHaveFocus(); +}); + +test("open combobox popover on arrow down", () => { + render(); + press.Tab(); + expect(screen.getByLabelText("Fruit")).toHaveFocus(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + press.ArrowDown(); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + expect(screen.getByText("Apple")).not.toHaveFocus(); +}); + +test("open combobox popover on arrow up", () => { + render(); + press.Tab(); + expect(screen.getByLabelText("Fruit")).toHaveFocus(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + press.ArrowUp(); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + expect(screen.getByText("Banana")).not.toHaveFocus(); +}); + +test("open combobox popover by typing on the combobox", () => { + render(); + press.Tab(); + expect(screen.getByLabelText("Fruit")).toHaveFocus(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + type("a"); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + expect(screen.getByText("Apple")).not.toHaveFocus(); +}); + +test("do not open combobox popover on arrow right/left", () => { + render(); + press.Tab(); + expect(screen.getByLabelText("Fruit")).toHaveFocus(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + press.ArrowLeft(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + press.ArrowRight(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); +}); + +test("do not open combobox popover on backspace on empty input", () => { + render(); + press.Tab(); + expect(screen.getByLabelText("Fruit")).toHaveFocus(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + type("\b"); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); +}); + +test("close combobox popover by clicking outside", () => { + const { baseElement } = render(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + click(screen.getByLabelText("Fruit")); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + click(baseElement); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); +}); + +test("close combobox popover by tabbing out", () => { + render( + <> + + + + ); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + click(screen.getByLabelText("Fruit")); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + press.ArrowDown(); + press.Tab(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + expect(screen.getByText("button")).toHaveFocus(); +}); + +test("close combobox popover by pressing esc", () => { + render(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + click(screen.getByLabelText("Fruit")); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + press.Escape(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + expect(screen.getByLabelText("Fruit")).toHaveFocus(); +}); + +test("open combobox popover after pressing esc", () => { + render(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + click(screen.getByLabelText("Fruit")); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + press.Escape(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + press.ArrowDown(); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + press.Escape(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + press.ArrowUp(); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + press.Escape(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + type("a"); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + press.Escape(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + press.ArrowDown(); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + press.Escape(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + press.ArrowDown(); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + press.Escape(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + type("\b"); + expect(screen.getByLabelText("Fruits")).toBeVisible(); +}); + +test("open combobox popover after pressing esc twice", () => { + render(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + click(screen.getByLabelText("Fruit")); + expect(screen.getByLabelText("Fruits")).toBeVisible(); + press.Escape(); + press.Escape(); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + press.ArrowDown(); + expect(screen.getByLabelText("Fruits")).toBeVisible(); +}); + +test("move through combobox options with keyboard", () => { + render(); + click(screen.getByLabelText("Fruit")); + press.ArrowDown(); + expect(screen.getByLabelText("Fruit")).toHaveFocus(); + expect(screen.getByText("Apple")).toHaveFocus(); + press.ArrowDown(); + expect(screen.getByText("Orange")).toHaveFocus(); + press.ArrowDown(); + expect(screen.getByText("Banana")).toHaveFocus(); + press.ArrowDown(); + expect(screen.getByLabelText("Fruit")).toHaveFocus(); + expect(screen.getByText("Apple")).not.toHaveFocus(); + expect(screen.getByText("Orange")).not.toHaveFocus(); + expect(screen.getByText("Banana")).not.toHaveFocus(); + press.ArrowUp(); + expect(screen.getByText("Banana")).toHaveFocus(); + press.ArrowUp(); + expect(screen.getByText("Orange")).toHaveFocus(); + press.ArrowUp(); + expect(screen.getByText("Apple")).toHaveFocus(); + press.ArrowUp(); + expect(screen.getByLabelText("Fruit")).toHaveFocus(); + expect(screen.getByText("Apple")).not.toHaveFocus(); + expect(screen.getByText("Orange")).not.toHaveFocus(); + expect(screen.getByText("Banana")).not.toHaveFocus(); + press.ArrowUp(); + expect(screen.getByText("Banana")).toHaveFocus(); + press.ArrowRight(); + expect(screen.getByText("Banana")).toHaveFocus(); + press.ArrowLeft(); + expect(screen.getByText("Banana")).toHaveFocus(); + press.Home(); + expect(screen.getByText("Banana")).toHaveFocus(); + press.End(); + expect(screen.getByText("Banana")).toHaveFocus(); +}); + +test("select combobox option by clicking on it", () => { + render(); + click(screen.getByLabelText("Fruit")); + expect(screen.getByLabelText("Fruit")).toHaveValue(""); + click(screen.getByText("Orange")); + expect(screen.getByLabelText("Fruit")).toHaveValue("Orange"); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + type("\b\b\b\b\b\ba"); + expect(screen.getByLabelText("Fruit")).toHaveValue("a"); + click(screen.getByText("Apple")); + expect(screen.getByLabelText("Fruit")).toHaveValue("Apple"); +}); + +test("select combobox option by pressing enter on it", () => { + render(); + click(screen.getByLabelText("Fruit")); + expect(screen.getByLabelText("Fruit")).toHaveValue(""); + press.ArrowUp(); + press.Enter(); + expect(screen.getByLabelText("Fruit")).toHaveValue("Banana"); + expect(screen.getByLabelText("Fruits")).not.toBeVisible(); + type("\b\b\b\b\b\ba"); + expect(screen.getByLabelText("Fruit")).toHaveValue("a"); + press.ArrowDown(); + press.Enter(); + expect(screen.getByLabelText("Fruit")).toHaveValue("Apple"); +}); + +test("do not select combobox option by pressing space on it", () => { + render(); + click(screen.getByLabelText("Fruit")); + expect(screen.getByLabelText("Fruit")).toHaveValue(""); + press.ArrowDown(); + press.Space(); + expect(screen.getByLabelText("Fruit")).toHaveValue(""); + expect(screen.getByLabelText("Fruits")).toBeVisible(); +}); + +test("unselect combobox option when typing on the combobox", () => { + render(); + click(screen.getByLabelText("Fruit")); + expect(screen.getByLabelText("Fruit")).toHaveValue(""); + press.ArrowDown(); + expect(screen.getByText("Apple")).toHaveFocus(); + type("a"); + expect(screen.getByText("Apple")).not.toHaveFocus(); + press.ArrowDown(); + expect(screen.getByText("Apple")).toHaveFocus(); +}); + +test("clicking on combobox input unselects combobox option", () => { + render(); + click(screen.getByLabelText("Fruit")); + press.ArrowDown(); + expect(screen.getByText("Apple")).toHaveFocus(); + click(screen.getByLabelText("Fruit")); + expect(screen.getByText("Apple")).not.toHaveFocus(); +}); diff --git a/packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/index.tsx b/packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/index.tsx new file mode 100644 index 00000000..76153826 --- /dev/null +++ b/packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/index.tsx @@ -0,0 +1,23 @@ +import * as React from "react"; +import { + unstable_useComboboxState as useComboboxState, + unstable_Combobox as Combobox, + unstable_ComboboxPopover as ComboboxPopover, + unstable_ComboboxOption as ComboboxOption, +} from "reakit/Combobox"; + +import "./style.css"; + +export default function AccessibleComboboxContext() { + const combobox = useComboboxState({ gutter: 8 }); + return ( + <> + + + + + + + + ); +} diff --git a/packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/style.css b/packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/style.css new file mode 100644 index 00000000..43809ce1 --- /dev/null +++ b/packages/reakit/src/Combobox/__examples__/AccessibleComboboxContext/style.css @@ -0,0 +1,72 @@ +:root { + --font-family: var(--font-family-body, sans-serif); + --font-size: var(--font-size-body, 16px); + --combobox-background: var(--color-background-50, #fff); + --combobox-color: var(--color-grayscale-700, #333); + --combobox-border: var(--color-alpha-500, rgba(0, 0, 0, 0.5)); + --combobox-border-focus: var(--color-primary-700, #1976d2); + --listbox-background: var(--color-background-200, #fff); + --listbox-color: var(--color-grayscale-700, #333); + --listbox-shadow-50: var(--color-shadow-50, rgba(0, 0, 0, 0.05)); + --listbox-shadow-100: var(--color-shadow-50, rgba(0, 0, 0, 0.1)); + --option-background-hover: var(--color-primary-50, #e3f2fd); + --option-background-focus: var(--color-primary-100, #bbdefb); +} + +[role="combobox"] { + font-family: var(--font-family); + font-size: var(--font-size); + background-color: var(--combobox-background); + color: var(--combobox-color); + border: 1px solid var(--combobox-border); + border-radius: 4px; + height: 2.5em; + padding: 0 1em; + outline: 0; + width: 250px; + box-sizing: border-box; +} + +[role="combobox"]:focus { + border-color: var(--combobox-border-focus); + box-shadow: 0 0 0 1px var(--combobox-border-focus); +} + +[role="listbox"] { + font-family: var(--font-family); + font-size: var(--font-size); + background-color: var(--listbox-background); + color: var(--listbox-color); + width: 250px; + z-index: 999; + padding: 1em; + box-sizing: border-box; + border-radius: 4px; + box-shadow: + 0 0 8px var(--listbox-shadow-50), + 0 10px 10px -5px var(--listbox-shadow-50), + 0 20px 25px -5px var(--listbox-shadow-100); +} + +[role="option"] { + cursor: default; + padding: 0.5em; + margin: 0 -0.5em; + border-radius: 4px; +} + +[role="option"]:first-child { + margin-top: -0.5em; +} + +[role="option"]:last-child { + margin-bottom: -0.5em; +} + +[role="option"]:hover { + background-color: var(--option-background-hover); +} + +[role="combobox"]:focus + [role="listbox"] [aria-selected="true"] { + background-color: var(--option-background-focus); +} diff --git a/packages/reakit/src/Combobox/__examples__/index.story.ts b/packages/reakit/src/Combobox/__examples__/index.story.ts index d9907ee8..b0e882c9 100644 --- a/packages/reakit/src/Combobox/__examples__/index.story.ts +++ b/packages/reakit/src/Combobox/__examples__/index.story.ts @@ -1,6 +1,7 @@ import { unstable_Combobox as Combobox } from "../Combobox"; export { default as AccessibleCombobox } from "./AccessibleCombobox"; +export { default as AccessibleComboboxContext } from "./AccessibleComboboxContext"; export { default as ComboboxAutoSelect } from "./ComboboxAutoSelect"; export { default as ComboboxBothAutocomplete } from "./ComboboxBothAutocomplete"; export { default as ComboboxBothAutoSelect } from "./ComboboxBothAutoSelect"; From 985f9df05d3d18107eb8211389bfc41a091e9109 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Tue, 9 Feb 2021 14:41:02 +0100 Subject: [PATCH 23/28] Remove old code --- packages/reakit-system/src/createComponent.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/reakit-system/src/createComponent.ts b/packages/reakit-system/src/createComponent.ts index 1fee9226..1fe5da23 100644 --- a/packages/reakit-system/src/createComponent.ts +++ b/packages/reakit-system/src/createComponent.ts @@ -74,16 +74,9 @@ export function createComponent({ const asKeys = as.render?.__keys || as.__keys; const asOptions = asKeys && splitProps(props, asKeys)[0]; - const allProps = - // If `as` is a string, then that means it's an element not a component - // we don't need to pass the state in this case. - // If a component is passed, then the component can decide - // what should happen with the state prop. - isPlainObject(props.state) && typeof as !== "string" - ? { state: props.state, ...elementProps } - : asOptions - ? { ...elementProps, ...asOptions } - : elementProps; + const allProps = asOptions + ? { ...elementProps, ...asOptions } + : elementProps; const element = useCreateElement(as, allProps as typeof props); if (wrapElement) { return wrapElement(element); From 171726f6d84a3322666416bfc69556b88281c6b5 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Tue, 9 Feb 2021 14:42:46 +0100 Subject: [PATCH 24/28] Remove old code --- packages/reakit-system/src/createComponent.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/reakit-system/src/createComponent.ts b/packages/reakit-system/src/createComponent.ts index 1fe5da23..5dc98e35 100644 --- a/packages/reakit-system/src/createComponent.ts +++ b/packages/reakit-system/src/createComponent.ts @@ -73,7 +73,6 @@ export function createComponent({ // @ts-ignore const asKeys = as.render?.__keys || as.__keys; const asOptions = asKeys && splitProps(props, asKeys)[0]; - const allProps = asOptions ? { ...elementProps, ...asOptions } : elementProps; From 3937cf95d33e510a004fcd716fd413d6d2418533 Mon Sep 17 00:00:00 2001 From: David Szabo Date: Tue, 9 Feb 2021 14:54:50 +0100 Subject: [PATCH 25/28] Checkin build files --- packages/reakit-playground/src/__deps/reakit.ts | 5 +++++ packages/reakit/.gitignore | 1 + 2 files changed, 6 insertions(+) diff --git a/packages/reakit-playground/src/__deps/reakit.ts b/packages/reakit-playground/src/__deps/reakit.ts index 7cc51879..073fa8c3 100644 --- a/packages/reakit-playground/src/__deps/reakit.ts +++ b/packages/reakit-playground/src/__deps/reakit.ts @@ -101,6 +101,11 @@ export default { "reakit/Rover/RoverState": require("reakit/Rover/RoverState"), "reakit/Separator": require("reakit/Separator"), "reakit/Separator/Separator": require("reakit/Separator/Separator"), + "reakit/StateContext": require("reakit/StateContext"), + "reakit/StateContext/createStateContext": require("reakit/StateContext/createStateContext"), + "reakit/StateContext/useStateContext": require("reakit/StateContext/useStateContext"), + "reakit/StateContext/useStateContextConsumer": require("reakit/StateContext/useStateContextConsumer"), + "reakit/StateContext/useStateContextProvider": require("reakit/StateContext/useStateContextProvider"), "reakit/Tab": require("reakit/Tab"), "reakit/Tab/Tab": require("reakit/Tab/Tab"), "reakit/Tab/TabList": require("reakit/Tab/TabList"), diff --git a/packages/reakit/.gitignore b/packages/reakit/.gitignore index f8e2a781..6a799af0 100644 --- a/packages/reakit/.gitignore +++ b/packages/reakit/.gitignore @@ -20,6 +20,7 @@ /Role /Rover /Separator +/StateContext /Tab /Tabbable /Toolbar From f9922e4fbae4d805820e0f267721d281a26c8c4e Mon Sep 17 00:00:00 2001 From: Eric Ribeiro Date: Sat, 17 Apr 2021 18:49:33 -0700 Subject: [PATCH 26/28] add new util functions --- .../reakit/src/Composite/__utils/fillGroups.ts | 18 +++++++++++++++++- .../Composite/__utils/findFirstEnabledItem.ts | 10 ++++++++++ .../src/Composite/__utils/getItemsInGroup.ts | 5 +++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/reakit/src/Composite/__utils/fillGroups.ts b/packages/reakit/src/Composite/__utils/fillGroups.ts index ec8ac772..374327d0 100644 --- a/packages/reakit/src/Composite/__utils/fillGroups.ts +++ b/packages/reakit/src/Composite/__utils/fillGroups.ts @@ -11,6 +11,22 @@ function createEmptyItem(groupId?: string) { }; } +// TODO: implement unit test +export function createEmptyItems(size: number, groupId?: string) { + const items = []; + + for (let i = 0; i++; i < size) { + items.push({ + id: "__EMPTY_ITEM__", + disabled: true, + ref: { current: null }, + groupId, + }); + } + + return items; +} + /** * Turns [[row1, row1], [row2]] into [[row1, row1], [row2, row2]] */ @@ -37,4 +53,4 @@ export function fillGroups( } return groups; -} +} \ No newline at end of file diff --git a/packages/reakit/src/Composite/__utils/findFirstEnabledItem.ts b/packages/reakit/src/Composite/__utils/findFirstEnabledItem.ts index f51d8490..a7a76965 100644 --- a/packages/reakit/src/Composite/__utils/findFirstEnabledItem.ts +++ b/packages/reakit/src/Composite/__utils/findFirstEnabledItem.ts @@ -6,3 +6,13 @@ export function findFirstEnabledItem(items: Item[], excludeId?: string) { } return items.find((item) => !item.disabled); } + +// TODO: define function's return type +export function findLastEnabledItem(items: Item[], excludeId?: string) { + if (excludeId) { + return items + .reverse() + .find((item) => !item.disabled && item.id !== excludeId); + } + return items.reverse().find((item) => !item.disabled); +} \ No newline at end of file diff --git a/packages/reakit/src/Composite/__utils/getItemsInGroup.ts b/packages/reakit/src/Composite/__utils/getItemsInGroup.ts index f58135b3..5b45040e 100644 --- a/packages/reakit/src/Composite/__utils/getItemsInGroup.ts +++ b/packages/reakit/src/Composite/__utils/getItemsInGroup.ts @@ -3,3 +3,8 @@ import { Item } from "./types"; export function getItemsInGroup(items: Item[], groupId?: string) { return items.filter((item) => item.groupId === groupId); } + +// TODO: implement unit test +export function getDisabledItems(items: Item[]) { + return items.filter((item) => item.disabled); +} From 2be1c8101fa744ecf1f18b8cfad3160aadd89fd5 Mon Sep 17 00:00:00 2001 From: Eric Ribeiro Date: Sat, 17 Apr 2021 18:56:05 -0700 Subject: [PATCH 27/28] fixed lint issues --- packages/reakit/src/Composite/__utils/fillGroups.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reakit/src/Composite/__utils/fillGroups.ts b/packages/reakit/src/Composite/__utils/fillGroups.ts index 374327d0..217fb0fb 100644 --- a/packages/reakit/src/Composite/__utils/fillGroups.ts +++ b/packages/reakit/src/Composite/__utils/fillGroups.ts @@ -53,4 +53,4 @@ export function fillGroups( } return groups; -} \ No newline at end of file +} From 266f2951ad3e575dbd9228047ca8bbd13c404375 Mon Sep 17 00:00:00 2001 From: Eric Ribeiro Date: Sat, 17 Apr 2021 19:35:45 -0700 Subject: [PATCH 28/28] fix yet another lint issue --- packages/reakit/src/Composite/__utils/findFirstEnabledItem.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reakit/src/Composite/__utils/findFirstEnabledItem.ts b/packages/reakit/src/Composite/__utils/findFirstEnabledItem.ts index a7a76965..90eb0708 100644 --- a/packages/reakit/src/Composite/__utils/findFirstEnabledItem.ts +++ b/packages/reakit/src/Composite/__utils/findFirstEnabledItem.ts @@ -15,4 +15,4 @@ export function findLastEnabledItem(items: Item[], excludeId?: string) { .find((item) => !item.disabled && item.id !== excludeId); } return items.reverse().find((item) => !item.disabled); -} \ No newline at end of file +}