From e4522ee040cfc85933d5defedf9b5ee14af44673 Mon Sep 17 00:00:00 2001 From: Jeff Luyau Date: Wed, 18 Dec 2024 16:51:54 -0800 Subject: [PATCH 01/23] GW coachmark --- packages/@react-spectrum/s2/package.json | 4 +- packages/@react-spectrum/s2/src/CoachMark.tsx | 541 ++++++++++++++++++ packages/@react-spectrum/s2/src/index.ts | 1 + .../s2/stories/CoachMark.stories.tsx | 202 +++++++ .../react-aria-components/src/CoachMark.tsx | 18 + packages/react-aria-components/src/index.ts | 1 + 6 files changed, 766 insertions(+), 1 deletion(-) create mode 100644 packages/@react-spectrum/s2/src/CoachMark.tsx create mode 100644 packages/@react-spectrum/s2/stories/CoachMark.stories.tsx create mode 100644 packages/react-aria-components/src/CoachMark.tsx diff --git a/packages/@react-spectrum/s2/package.json b/packages/@react-spectrum/s2/package.json index 589eb2b82dc..35cc2cb6451 100644 --- a/packages/@react-spectrum/s2/package.json +++ b/packages/@react-spectrum/s2/package.json @@ -128,6 +128,7 @@ "@react-aria/collections": "3.0.0-alpha.6", "@react-aria/i18n": "^3.12.4", "@react-aria/interactions": "^3.22.5", + "@react-aria/overlays": "^3.24.0", "@react-aria/utils": "^3.26.0", "@react-spectrum/utils": "^3.12.0", "@react-stately/layout": "^4.1.0", @@ -142,7 +143,8 @@ "@react-types/textfield": "^3.10.0", "csstype": "^3.0.2", "react-aria": "^3.36.0", - "react-aria-components": "^1.5.0" + "react-aria-components": "^1.5.0", + "react-stately": "^3.34.0" }, "peerDependencies": { "@testing-library/react": "^15.0.7", diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx new file mode 100644 index 00000000000..a97c7886876 --- /dev/null +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -0,0 +1,541 @@ + +/* + * Copyright 2024 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ +import {ActionMenuContext} from './ActionMenu'; +import { + DialogTrigger as AriaDialogTrigger, + DialogTriggerProps as AriaDialogTriggerProps, + Popover as AriaPopover, + DEFAULT_SLOT, + DialogContext, + OverlayTriggerStateContext, + PopoverContext, + PopoverProps, + Provider, + RootMenuTriggerStateContext +} from 'react-aria-components'; +import {ButtonContext} from './Button'; +import {Card} from './Card'; +import {CheckboxContext} from './Checkbox'; +import {colorScheme, getAllowedOverrides} from './style-utils' with {type: 'macro'}; +import {ColorSchemeContext} from './Provider'; +import {ContentContext, FooterContext, KeyboardContext, TextContext} from './Content'; +import {DividerContext} from './Divider'; +import {FocusScope, useId, useOverlayTrigger} from 'react-aria'; +import {ForwardedRef, forwardRef, ReactNode, useContext, useRef, useState} from 'react'; +import {forwardRefType} from './types'; +import {ImageContext} from './Image'; +import {ImageCoordinator} from './ImageCoordinator'; +import {keyframes, raw} from '../style/style-macro' with {type: 'macro'}; +import {mergeStyles} from '../style/runtime'; +import {PressResponder} from '@react-aria/interactions'; +import {SliderContext} from './Slider'; +import {space, style} from '../style' with {type: 'macro'}; +import {useMenuTriggerState} from 'react-stately'; + +export interface CoachMarkProps extends PopoverProps {} + +const fadeKeyframes = keyframes(` + from { + opacity: 0; + } + + to { + opacity: 1; + } +`); +const slideUpKeyframes = keyframes(` + from { + transform: translateY(-4px); + } + + to { + transform: translateY(0); + } +`); +const slideDownKeyframes = keyframes(` + from { + transform: translateY(4px); + } + + to { + transform: translateY(0); + } +`); +const slideRightKeyframes = keyframes(` + from { + transform: translateX(4px); + } + + to { + transform: translateX(0); + } +`); +const slideLeftKeyframes = keyframes(` + from { + transform: translateX(-4px); + } + + to { + transform: translateX(0); + } +`); + +let popover = style({ + ...colorScheme(), + '--s2-container-bg': { + type: 'backgroundColor', + value: 'layer-2' + }, + backgroundColor: '--s2-container-bg', + borderRadius: 'lg', + filter: { + isArrowShown: 'elevated' + }, + // Use box-shadow instead of filter when an arrow is not shown. + // This fixes the shadow stacking problem with submenus. + boxShadow: { + default: 'elevated', + isArrowShown: 'none' + }, + borderStyle: 'solid', + borderWidth: 1, + borderColor: { + default: 'gray-200', + forcedColors: 'ButtonBorder' + }, + width: { + size: { + // Copied from designs, not sure if correct. + S: 336, + M: 416, + L: 576 + } + }, + // Don't be larger than full screen minus 2 * containerPadding + maxWidth: '[calc(100vw - 24px)]', + boxSizing: 'border-box', + translateY: { + placement: { + bottom: { + isArrowShown: 8 // TODO: not defined yet should this change with font size? need boolean support for 'hideArrow' prop + }, + top: { + isArrowShown: -8 + } + } + }, + translateX: { + placement: { + left: { + isArrowShown: -8 + }, + right: { + isArrowShown: 8 + } + } + }, + animation: { + placement: { + top: { + isEntering: `${slideDownKeyframes}, ${fadeKeyframes}`, + isExiting: `${slideDownKeyframes}, ${fadeKeyframes}` + }, + bottom: { + isEntering: `${slideUpKeyframes}, ${fadeKeyframes}`, + isExiting: `${slideUpKeyframes}, ${fadeKeyframes}` + }, + left: { + isEntering: `${slideRightKeyframes}, ${fadeKeyframes}`, + isExiting: `${slideRightKeyframes}, ${fadeKeyframes}` + }, + right: { + isEntering: `${slideLeftKeyframes}, ${fadeKeyframes}`, + isExiting: `${slideLeftKeyframes}, ${fadeKeyframes}` + } + }, + isSubmenu: { + isEntering: fadeKeyframes, + isExiting: fadeKeyframes + } + }, + animationDuration: { + isEntering: 200, + isExiting: 200 + }, + animationDirection: { + isEntering: 'normal', + isExiting: 'reverse' + }, + animationTimingFunction: { + isExiting: 'in' + }, + transition: '[opacity, transform]', + willChange: '[opacity, transform]', + isolation: 'isolate', + pointerEvents: { + isExiting: 'none' + } +}, getAllowedOverrides()); + +const image = style({ + width: 'full', + aspectRatio: '[3/2]', + objectFit: 'cover', + userSelect: 'none', + pointerEvents: 'none' +}); + +let title = style({ + font: 'title', + fontSize: { + size: { + XS: 'title-xs', + S: 'title-xs', + M: 'title-sm', + L: 'title', + XL: 'title-lg' + } + }, + lineClamp: 3, + gridArea: 'title' +}); + +let description = style({ + font: 'body', + fontSize: { + size: { + XS: 'body-2xs', + S: 'body-2xs', + M: 'body-xs', + L: 'body-sm', + XL: 'body' + } + }, + lineClamp: 3, + gridArea: 'description' +}); + +let keyboard = style({ + gridArea: 'keyboard', + font: 'ui', + fontWeight: 'light', + color: 'gray-600', + background: 'gray-25', + unicodeBidi: 'plaintext' +}); + +let steps = style({ + font: 'detail', + fontSize: 'detail-sm', + alignSelf: 'center' +}); + +let content = style({ + display: 'grid', + // By default, all elements are displayed in a stack. + // If an action menu is present, place it next to the title. + gridTemplateColumns: { + default: ['1fr'], + ':has([data-slot=menu])': ['minmax(0, 1fr)', 'auto'] + }, + gridTemplateAreas: { + default: [ + 'title keyboard', + 'description keyboard' + ], + ':has([data-slot=menu])': [ + 'title menu', + 'keyboard keyboard', + 'description description' + ] + }, + columnGap: 4, + flexGrow: 1, + alignItems: 'baseline', + alignContent: 'space-between', + rowGap: { + size: { + XS: 4, + S: 4, + M: space(6), + L: space(6), + XL: 8 + } + }, + paddingTop: { + default: '--card-spacing', + ':first-child': 0 + }, + paddingBottom: { + default: '[calc(var(--card-spacing) * 1.5 / 2)]', + ':last-child': 0 + } +}); + +let actionMenu = style({ + gridArea: 'menu', + // Don't cause the row to expand, preserve gap between title and description text. + // Would use -100% here but it doesn't work in Firefox. + marginY: '[calc(-1 * self(height))]' +}); + +let footer = style({ + display: 'flex', + flexDirection: 'row', + alignItems: 'end', + justifyContent: 'space-between', + gap: 8, + paddingTop: '[calc(var(--card-spacing) * 1.5 / 2)]' +}); + +const actionButtonSize = { + XS: 'XS', + S: 'XS', + M: 'S', + L: 'M', + XL: 'L' +} as const; + +export function CoachMark(props: CoachMarkProps) { + let colorScheme = useContext(ColorSchemeContext); + let {density = 'regular', size = 'M', variant = 'primary', UNSAFE_className = '', UNSAFE_style, styles, id, ...otherProps} = props; + + let children = ( + + + {typeof props.children === 'function' ? props.children({size}) : props.children} + + + ); + return ( + mergeStyles(popover({...renderProps, colorScheme}))}> + + {/* }// Reset OverlayTriggerStateContext so the buttons inside the dialog don't retain their hover state. */} + + {children} + + + + ); +} + + +export interface CoachMarkTriggerProps extends AriaDialogTriggerProps { +} + +/** + * DialogTrigger serves as a wrapper around a Dialog and its associated trigger, linking the Dialog's + * open state with the trigger's press state. Additionally, it allows you to customize the type and + * positioning of the Dialog. + */ +export function CoachMarkTrigger(props: CoachMarkTriggerProps) { + // Use useMenuTriggerState instead of useOverlayTriggerState in case a menu is embedded in the dialog. + // This is needed to handle submenus. + let state = useMenuTriggerState(props); + + let containerRef = useRef(null); + let {triggerProps, overlayProps} = useOverlayTrigger({type: 'dialog'}, state, containerRef); + + // Label dialog by the trigger as a fallback if there is no title slot. + // This is done in RAC instead of hooks because otherwise we cannot distinguish + // between context and props. Normally aria-labelledby overrides the title + // but when sent by context we want the title to win. + triggerProps.id = useId(); + overlayProps['aria-labelledby'] = triggerProps.id; + console.log('containerRef', containerRef); + + + return ( + + + + {props.children} + + + + ); +} +// export function CoachMarkTrigger(props: CoachMarkTriggerProps) { +// return ( +// +// {/* RAC sets isPressed via PressResponder when the dialog is open. +// We don't want press scaling to appear to get "stuck", so override this. */} +// +// +// {props.children} +// +// +// +// ); +// } + + +// TODO better way to calculate 4px transform? (not 4%?) +const pulseAnimation = keyframes(` + 0% { + box-shadow: 0 0 0 4px rgba(20, 115, 230, 0.40); + transform: scale(calc(100%)); + } + 50% { + box-shadow: 0 0 0 10px rgba(20, 115, 230, 0.20); + transform: scale(104%); + } + 100% { + box-shadow: 0 0 0 4px rgba(20, 115, 230, 0.40); + transform: scale(calc(100%)); + } +`); + + +const indicator = style({ + animationDuration: 1000, + animationIterationCount: 'infinite', + animationFillMode: 'forwards', + animationTimingFunction: 'in-out', + position: 'relative', + '--activeElement': { + type: 'outlineColor', + value: { + default: 'focus-ring', + forcedColors: 'Highlight' + } + }, + '--borderOffset': { + type: 'top', + value: { + default: '[-2px]', + ':has([data-trigger=checkbox])': '[-6px]', + ':has([data-trigger=slider])': '[-8px]', + offset: { + M: '[-6px]', + L: '[-8px]' + } + } + }, + '--ringRadius': { + type: 'top', // is there a generic for pixel values? + value: { + default: '[10px]', + ':has([data-trigger=button])': '[18px]', + ':has([data-trigger=checkbox])': '[6px]' + } + } +}); + +const pulse = raw(`&:before { content: ""; display: inline-block; position: absolute; top: var(--borderOffset); bottom: var(--borderOffset); left: var(--borderOffset); right: var(--borderOffset); border-radius: var(--ringRadius); outline-style: solid; outline-color: var(--activeElement); outline-width: 4px; animation-duration: 2s; animation-iteration-count: infinite; animation-timing-function: ease-in-out; animation-fill-mode: forwards; animation-name: ${pulseAnimation}}`); + +interface CoachIndicatorProps { + children: ReactNode, + isActive?: boolean +} +export const CoachIndicator = /*#__PURE__*/ (forwardRef as forwardRefType)(function CoachIndicator(props: CoachIndicatorProps, ref: ForwardedRef) { +// export const CoachIndicator = forwardRef((props, ref) => { + const {children, isActive} = props; + + return ( +
+ + {children} + +
+ ); +}); + +// FUTURE: support multiple tours on a page? +export function useTour({defaultStep = 1, defaultComplete = false}) { + const [isComplete, setIsComplete] = useState(defaultComplete); + const [currentStep, setCurrentStep] = useState(defaultStep); + const [totalSteps, setTotalSteps] = useState(0); + const advanceStep = () => { + if (currentStep + 1 > totalSteps) { + setIsComplete(true); + } + setCurrentStep(currentStep < totalSteps ? currentStep + 1 : totalSteps); + }; + const previousStep = () => setCurrentStep(currentStep > 0 ? currentStep - 1 : 0); + const skipTour = () => setIsComplete(true); + const restartTour = () => { + setIsComplete(false); + setCurrentStep(1); + }; + + const coachMarkTriggerProps = (step: number) => { + // TODO find a better way for this so it's not run on every rerender + if (step > totalSteps) { + setTotalSteps(step); + } + return { + isOpen: !isComplete && step === currentStep + }; + }; + + return { + coachMarkTriggerProps, + currentStep, + totalSteps, + advanceStep, + previousStep, + skipTour, + restartTour + }; +} diff --git a/packages/@react-spectrum/s2/src/index.ts b/packages/@react-spectrum/s2/src/index.ts index 72da80811e9..9d785f74b7f 100644 --- a/packages/@react-spectrum/s2/src/index.ts +++ b/packages/@react-spectrum/s2/src/index.ts @@ -26,6 +26,7 @@ export {CardView, CardViewContext} from './CardView'; export {Checkbox, CheckboxContext} from './Checkbox'; export {CheckboxGroup, CheckboxGroupContext} from './CheckboxGroup'; export {CloseButton} from './CloseButton'; +export {CoachMark, CoachMarkTrigger, CoachIndicator, useTour} from './CoachMark'; export {ColorArea, ColorAreaContext} from './ColorArea'; export {ColorField, ColorFieldContext} from './ColorField'; export {ColorSlider, ColorSliderContext} from './ColorSlider'; diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx new file mode 100644 index 00000000000..7bd4bdbcf5b --- /dev/null +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -0,0 +1,202 @@ +/* + * Copyright 2024 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import { + ActionButton, ActionMenu, + Button, + CardPreview, + Checkbox, + CoachIndicator, + CoachMark, + CoachMarkTrigger, + Content, Footer, Image, Keyboard, MenuItem, + Slider, StatusLight, + Text, useTour +} from '../src'; +import Filter from '../s2wf-icons/S2_Icon_Filter_20_N.svg'; +import type {Meta} from '@storybook/react'; +import {style} from '../style' with {type: 'macro'}; +import TextAdd from '../s2wf-icons/S2_Icon_TextAdd_20_N.svg'; +import TextBold from '../s2wf-icons/S2_Icon_TextBold_20_N.svg'; +import TextIcon from '../s2wf-icons/S2_Icon_Text_20_N.svg'; +import {Toolbar} from 'react-aria-components'; + +const meta: Meta = { + component: CoachMark, + parameters: { + layout: 'centered' + }, + tags: ['autodocs'], + argTypes: { + placement: { + control: 'radio', + options: ['top', 'left', 'left top', 'right', 'right top', 'bottom'] + } + }, + title: 'CoachMark' +}; + +export default meta; + +export const CoachMarkExample = (args) => ( + + Sync with CC + + + + + + Hello + + Skip tour + Restart tour + + Command + B + This is the description + +
+ 1 of 10 + + +
+
+
+); + +function CoachMarkBase({step, currentStep, totalSteps, description = '', skipTour, restartTour, advanceStep, previousStep, hasPressAction = false, placement = 'right top'}) { + const onAction = actionKey => { + if (actionKey === 'skip') { + skipTour(); + } else if (actionKey === 'restart') { + restartTour(); + } + }; + return ( + + + + + + Coach mark title + + Skip tour + {step !== 1 && Restart tour} + + Command + B + {description || 'This is the description'} + +
+ {currentStep} of {totalSteps} + {step !== 1 && } + {step !== totalSteps && !hasPressAction && } + {step === totalSteps && !hasPressAction && } +
+
+ ); +} + +export const TourExample = () => { + let {coachMarkTriggerProps, ...otherTourProps} = useTour({}); + return (
+
+ + + + + + + + + + + + + + Sync with CC + + +
+
+
+ + + + + + + + + + + + + + + + + + + +
+
); +}; + +// export const Test = () => ( +// ); +// +// export const Indicator = (args: any) => (
); +export const IndicatorButton = (args: any) => ( + +); + +export const IndicatorActionButton = (args: any) => ( + + + +); + +export const IndicatorToolbar = (args: any) => ( + + + + + + + + + + + +); + +export const IndicatorSlider = (args: any) => ( + +); + +export const IndicatorCheckbox = (args: any) => ( + Sync with CC +); + +// export const LongLabel = (args: any) => (Checkbox with very long label so we can see wrapping); diff --git a/packages/react-aria-components/src/CoachMark.tsx b/packages/react-aria-components/src/CoachMark.tsx new file mode 100644 index 00000000000..ad2fb895736 --- /dev/null +++ b/packages/react-aria-components/src/CoachMark.tsx @@ -0,0 +1,18 @@ + +export function CoachMark(props) { + return ( +
+ ); +} + +export function CoachIndicator(props) { + return ( +
+ ); +} + +export function Tour(props) { + return ( +
+ ); +} diff --git a/packages/react-aria-components/src/index.ts b/packages/react-aria-components/src/index.ts index 18cc7de8814..e82b83dc2c9 100644 --- a/packages/react-aria-components/src/index.ts +++ b/packages/react-aria-components/src/index.ts @@ -21,6 +21,7 @@ export {Breadcrumbs, BreadcrumbsContext, Breadcrumb} from './Breadcrumbs'; export {Button, ButtonContext} from './Button'; export {Calendar, CalendarGrid, CalendarGridHeader, CalendarGridBody, CalendarHeaderCell, CalendarCell, RangeCalendar, CalendarContext, RangeCalendarContext, CalendarStateContext, RangeCalendarStateContext} from './Calendar'; export {Checkbox, CheckboxGroup, CheckboxGroupContext, CheckboxGroupStateContext} from './Checkbox'; +export {CoachMark, CoachIndicator, Tour} from './CoachMark'; export {ColorArea, ColorAreaStateContext} from './ColorArea'; export {ColorField, ColorFieldStateContext} from './ColorField'; export {ColorPicker, ColorPickerContext, ColorPickerStateContext} from './ColorPicker'; From d42a8ee01134c1ab0c87062f9fe717d98500fbd9 Mon Sep 17 00:00:00 2001 From: Jeff Luyau Date: Thu, 9 Jan 2025 10:24:00 -0800 Subject: [PATCH 02/23] fix TS errors --- packages/@react-spectrum/s2/src/CoachMark.tsx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index a97c7886876..1eccb0eddf3 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -12,7 +12,6 @@ */ import {ActionMenuContext} from './ActionMenu'; import { - DialogTrigger as AriaDialogTrigger, DialogTriggerProps as AriaDialogTriggerProps, Popover as AriaPopover, DEFAULT_SLOT, @@ -30,7 +29,6 @@ import {colorScheme, getAllowedOverrides} from './style-utils' with {type: 'macr import {ColorSchemeContext} from './Provider'; import {ContentContext, FooterContext, KeyboardContext, TextContext} from './Content'; import {DividerContext} from './Divider'; -import {FocusScope, useId, useOverlayTrigger} from 'react-aria'; import {ForwardedRef, forwardRef, ReactNode, useContext, useRef, useState} from 'react'; import {forwardRefType} from './types'; import {ImageContext} from './Image'; @@ -40,9 +38,15 @@ import {mergeStyles} from '../style/runtime'; import {PressResponder} from '@react-aria/interactions'; import {SliderContext} from './Slider'; import {space, style} from '../style' with {type: 'macro'}; +import {useId, useOverlayTrigger} from 'react-aria'; import {useMenuTriggerState} from 'react-stately'; -export interface CoachMarkProps extends PopoverProps {} +export interface CoachMarkProps extends Omit { + /** The children of the coach mark. */ + children: ReactNode | ((renderProps: { size: 'XS' | 'S' | 'M' | 'L' | 'XL' }) => ReactNode), + + size: 'S' | 'M' | 'L' | 'XL' +} const fadeKeyframes = keyframes(` from { @@ -308,7 +312,7 @@ const actionButtonSize = { export function CoachMark(props: CoachMarkProps) { let colorScheme = useContext(ColorSchemeContext); - let {density = 'regular', size = 'M', variant = 'primary', UNSAFE_className = '', UNSAFE_style, styles, id, ...otherProps} = props; + let {size = 'M'} = props; let children = ( (null); + let containerRef = useRef(null); let {triggerProps, overlayProps} = useOverlayTrigger({type: 'dialog'}, state, containerRef); // Label dialog by the trigger as a fallback if there is no title slot. @@ -382,7 +386,6 @@ export function CoachMarkTrigger(props: CoachMarkTriggerProps) { // but when sent by context we want the title to win. triggerProps.id = useId(); overlayProps['aria-labelledby'] = triggerProps.id; - console.log('containerRef', containerRef); return ( @@ -474,7 +477,7 @@ interface CoachIndicatorProps { children: ReactNode, isActive?: boolean } -export const CoachIndicator = /*#__PURE__*/ (forwardRef as forwardRefType)(function CoachIndicator(props: CoachIndicatorProps, ref: ForwardedRef) { +export const CoachIndicator = /*#__PURE__*/ (forwardRef as forwardRefType)(function CoachIndicator(props: CoachIndicatorProps, ref: ForwardedRef) { // export const CoachIndicator = forwardRef((props, ref) => { const {children, isActive} = props; From 3b7f7bf136b6474fb66f84b40088138b306f8af3 Mon Sep 17 00:00:00 2001 From: Jeff Luyau Date: Thu, 9 Jan 2025 10:51:20 -0800 Subject: [PATCH 03/23] remove unrelated files, update yarn.lock --- .../@react-aria/combobox/src/useSearchMenu.ts | 380 ------------------ packages/@react-spectrum/s2/package.json | 4 +- packages/@react-spectrum/s2/src/CoachMark.tsx | 2 +- .../react-aria-components/src/CoachMark.tsx | 18 - .../react-aria-components/src/SearchMenu.tsx | 0 .../stories/SearchMenu.stories.tsx | 30 -- yarn.lock | 1 + 7 files changed, 4 insertions(+), 431 deletions(-) delete mode 100644 packages/@react-aria/combobox/src/useSearchMenu.ts delete mode 100644 packages/react-aria-components/src/CoachMark.tsx delete mode 100644 packages/react-aria-components/src/SearchMenu.tsx delete mode 100644 packages/react-aria-components/stories/SearchMenu.stories.tsx diff --git a/packages/@react-aria/combobox/src/useSearchMenu.ts b/packages/@react-aria/combobox/src/useSearchMenu.ts deleted file mode 100644 index 4cf9785195d..00000000000 --- a/packages/@react-aria/combobox/src/useSearchMenu.ts +++ /dev/null @@ -1,380 +0,0 @@ -/* - * Copyright 2020 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ - -import {announce} from '@react-aria/live-announcer'; -import {AriaButtonProps} from '@react-types/button'; -import {AriaComboBoxProps} from '@react-types/combobox'; -import {ariaHideOutside} from '@react-aria/overlays'; -import {AriaListBoxOptions, getItemId, listData} from '@react-aria/listbox'; -import {BaseEvent, DOMAttributes, KeyboardDelegate, LayoutDelegate, PressEvent, RefObject, RouterOptions, ValidationResult} from '@react-types/shared'; -import {chain, isAppleDevice, mergeProps, useLabels, useRouter} from '@react-aria/utils'; -import {ComboBoxState} from '@react-stately/combobox'; -import {FocusEvent, InputHTMLAttributes, KeyboardEvent, TouchEvent, useEffect, useMemo, useRef} from 'react'; -import {getChildNodes, getItemCount} from '@react-stately/collections'; -// @ts-ignore -import intlMessages from '../intl/*.json'; -import {ListKeyboardDelegate, useSelectableCollection} from '@react-aria/selection'; -import {privateValidationStateProp} from '@react-stately/form'; -import {useLocalizedStringFormatter} from '@react-aria/i18n'; -import {useMenuTrigger} from '@react-aria/menu'; -import {useTextField} from '@react-aria/textfield'; - -export interface AriaComboBoxOptions extends Omit, 'children'> { - /** The ref for the input element. */ - inputRef: RefObject, - /** The ref for the list box popover. */ - popoverRef: RefObject, - /** The ref for the list box. */ - listBoxRef: RefObject, - /** The ref for the optional list box popup trigger button. */ - buttonRef?: RefObject, - /** An optional keyboard delegate implementation, to override the default. */ - keyboardDelegate?: KeyboardDelegate, - /** - * A delegate object that provides layout information for items in the collection. - * By default this uses the DOM, but this can be overridden to implement things like - * virtualized scrolling. - */ - layoutDelegate?: LayoutDelegate -} - -export interface ComboBoxAria extends ValidationResult { - /** Props for the label element. */ - labelProps: DOMAttributes, - /** Props for the combo box input element. */ - inputProps: InputHTMLAttributes, - /** Props for the list box, to be passed to [useListBox](useListBox.html). */ - listBoxProps: AriaListBoxOptions, - /** Props for the optional trigger button, to be passed to [useButton](useButton.html). */ - buttonProps: AriaButtonProps, - /** Props for the combo box description element, if any. */ - descriptionProps: DOMAttributes, - /** Props for the combo box error message element, if any. */ - errorMessageProps: DOMAttributes -} - -/** - * Provides the behavior and accessibility implementation for a combo box component. - * A combo box combines a text input with a listbox, allowing users to filter a list of options to items matching a query. - * @param props - Props for the combo box. - * @param state - State for the select, as returned by `useComboBoxState`. - */ -export function useComboBox(props: AriaComboBoxOptions, state: ComboBoxState): ComboBoxAria { - let { - buttonRef, - popoverRef, - inputRef, - listBoxRef, - keyboardDelegate, - layoutDelegate, - // completionMode = 'suggest', - shouldFocusWrap, - isReadOnly, - isDisabled - } = props; - - let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/combobox'); - let {menuTriggerProps, menuProps} = useMenuTrigger( - { - type: 'listbox', - isDisabled: isDisabled || isReadOnly - }, - state, - buttonRef - ); - - // Set listbox id so it can be used when calling getItemId later - listData.set(state, {id: menuProps.id}); - - // By default, a KeyboardDelegate is provided which uses the DOM to query layout information (e.g. for page up/page down). - // When virtualized, the layout object will be passed in as a prop and override this. - let {collection} = state; - let {disabledKeys} = state.selectionManager; - let delegate = useMemo(() => ( - keyboardDelegate || new ListKeyboardDelegate({ - collection, - disabledKeys, - ref: listBoxRef, - layoutDelegate - }) - ), [keyboardDelegate, layoutDelegate, collection, disabledKeys, listBoxRef]); - - // Use useSelectableCollection to get the keyboard handlers to apply to the textfield - let {collectionProps} = useSelectableCollection({ - selectionManager: state.selectionManager, - keyboardDelegate: delegate, - disallowTypeAhead: true, - disallowEmptySelection: true, - shouldFocusWrap, - ref: inputRef, - // Prevent item scroll behavior from being applied here, should be handled in the user's Popover + ListBox component - isVirtualized: true - }); - - let router = useRouter(); - - // For textfield specific keydown operations - let onKeyDown = (e: BaseEvent>) => { - if (e.nativeEvent.isComposing) { - return; - } - switch (e.key) { - case 'Enter': - case 'Tab': - // Prevent form submission if menu is open since we may be selecting a option - if (state.isOpen && e.key === 'Enter') { - e.preventDefault(); - } - - // If the focused item is a link, trigger opening it. Items that are links are not selectable. - if (state.isOpen && state.selectionManager.focusedKey != null && state.selectionManager.isLink(state.selectionManager.focusedKey)) { - if (e.key === 'Enter') { - let item = listBoxRef.current.querySelector(`[data-key="${CSS.escape(state.selectionManager.focusedKey.toString())}"]`); - if (item instanceof HTMLAnchorElement) { - let collectionItem = state.collection.getItem(state.selectionManager.focusedKey); - router.open(item, e, collectionItem.props.href, collectionItem.props.routerOptions as RouterOptions); - } - } - - state.close(); - } else { - state.commit(); - } - break; - case 'Escape': - if ( - state.selectedKey !== null || - state.inputValue === '' || - props.allowsCustomValue - ) { - e.continuePropagation(); - } - state.revert(); - break; - case 'ArrowDown': - state.open('first', 'manual'); - break; - case 'ArrowUp': - state.open('last', 'manual'); - break; - case 'ArrowLeft': - case 'ArrowRight': - state.selectionManager.setFocusedKey(null); - break; - } - }; - - let onBlur = (e: FocusEvent) => { - let blurFromButton = buttonRef?.current && buttonRef.current === e.relatedTarget; - let blurIntoPopover = popoverRef.current?.contains(e.relatedTarget); - // Ignore blur if focused moved to the button(if exists) or into the popover. - if (blurFromButton || blurIntoPopover) { - return; - } - - if (props.onBlur) { - props.onBlur(e); - } - - state.setFocused(false); - }; - - let onFocus = (e: FocusEvent) => { - if (state.isFocused) { - return; - } - - if (props.onFocus) { - props.onFocus(e); - } - - state.setFocused(true); - }; - - let {isInvalid, validationErrors, validationDetails} = state.displayValidation; - let {labelProps, inputProps, descriptionProps, errorMessageProps} = useTextField({ - ...props, - onChange: state.setInputValue, - onKeyDown: !isReadOnly ? chain(state.isOpen && collectionProps.onKeyDown, onKeyDown, props.onKeyDown) : props.onKeyDown, - onBlur, - value: state.inputValue, - onFocus, - autoComplete: 'off', - validate: undefined, - [privateValidationStateProp]: state - }, inputRef); - - // Press handlers for the ComboBox button - let onPress = (e: PressEvent) => { - if (e.pointerType === 'touch') { - // Focus the input field in case it isn't focused yet - inputRef.current.focus(); - state.toggle(null, 'manual'); - } - }; - - let onPressStart = (e: PressEvent) => { - if (e.pointerType !== 'touch') { - inputRef.current.focus(); - state.toggle((e.pointerType === 'keyboard' || e.pointerType === 'virtual') ? 'first' : null, 'manual'); - } - }; - - let triggerLabelProps = useLabels({ - id: menuTriggerProps.id, - 'aria-label': stringFormatter.format('buttonLabel'), - 'aria-labelledby': props['aria-labelledby'] || labelProps.id - }); - - let listBoxProps = useLabels({ - id: menuProps.id, - 'aria-label': stringFormatter.format('listboxLabel'), - 'aria-labelledby': props['aria-labelledby'] || labelProps.id - }); - - // If a touch happens on direct center of ComboBox input, might be virtual click from iPad so open ComboBox menu - let lastEventTime = useRef(0); - let onTouchEnd = (e: TouchEvent) => { - if (isDisabled || isReadOnly) { - return; - } - - // Sometimes VoiceOver on iOS fires two touchend events in quick succession. Ignore the second one. - if (e.timeStamp - lastEventTime.current < 500) { - e.preventDefault(); - inputRef.current.focus(); - return; - } - - let rect = (e.target as Element).getBoundingClientRect(); - let touch = e.changedTouches[0]; - - let centerX = Math.ceil(rect.left + .5 * rect.width); - let centerY = Math.ceil(rect.top + .5 * rect.height); - - if (touch.clientX === centerX && touch.clientY === centerY) { - e.preventDefault(); - inputRef.current.focus(); - state.toggle(null, 'manual'); - - lastEventTime.current = e.timeStamp; - } - }; - - // VoiceOver has issues with announcing aria-activedescendant properly on change - // (especially on iOS). We use a live region announcer to announce focus changes - // manually. In addition, section titles are announced when navigating into a new section. - let focusedItem = state.selectionManager.focusedKey != null && state.isOpen - ? state.collection.getItem(state.selectionManager.focusedKey) - : undefined; - let sectionKey = focusedItem?.parentKey ?? null; - let itemKey = state.selectionManager.focusedKey ?? null; - let lastSection = useRef(sectionKey); - let lastItem = useRef(itemKey); - useEffect(() => { - if (isAppleDevice() && focusedItem != null && itemKey !== lastItem.current) { - let isSelected = state.selectionManager.isSelected(itemKey); - let section = sectionKey != null ? state.collection.getItem(sectionKey) : null; - let sectionTitle = section?.['aria-label'] || (typeof section?.rendered === 'string' ? section.rendered : '') || ''; - - let announcement = stringFormatter.format('focusAnnouncement', { - isGroupChange: section && sectionKey !== lastSection.current, - groupTitle: sectionTitle, - groupCount: section ? [...getChildNodes(section, state.collection)].length : 0, - optionText: focusedItem['aria-label'] || focusedItem.textValue || '', - isSelected - }); - - announce(announcement); - } - - lastSection.current = sectionKey; - lastItem.current = itemKey; - }); - - // Announce the number of available suggestions when it changes - let optionCount = getItemCount(state.collection); - let lastSize = useRef(optionCount); - let lastOpen = useRef(state.isOpen); - useEffect(() => { - // Only announce the number of options available when the menu opens if there is no - // focused item, otherwise screen readers will typically read e.g. "1 of 6". - // The exception is VoiceOver since this isn't included in the message above. - let didOpenWithoutFocusedItem = - state.isOpen !== lastOpen.current && - (state.selectionManager.focusedKey == null || isAppleDevice()); - - if (state.isOpen && (didOpenWithoutFocusedItem || optionCount !== lastSize.current)) { - let announcement = stringFormatter.format('countAnnouncement', {optionCount}); - announce(announcement); - } - - lastSize.current = optionCount; - lastOpen.current = state.isOpen; - }); - - // Announce when a selection occurs for VoiceOver. Other screen readers typically do this automatically. - let lastSelectedKey = useRef(state.selectedKey); - useEffect(() => { - if (isAppleDevice() && state.isFocused && state.selectedItem && state.selectedKey !== lastSelectedKey.current) { - let optionText = state.selectedItem['aria-label'] || state.selectedItem.textValue || ''; - let announcement = stringFormatter.format('selectedAnnouncement', {optionText}); - announce(announcement); - } - - lastSelectedKey.current = state.selectedKey; - }); - - useEffect(() => { - if (state.isOpen) { - return ariaHideOutside([inputRef.current, popoverRef.current]); - } - }, [state.isOpen, inputRef, popoverRef]); - - return { - labelProps, - buttonProps: { - ...menuTriggerProps, - ...triggerLabelProps, - excludeFromTabOrder: true, - preventFocusOnPress: true, - onPress, - onPressStart, - isDisabled: isDisabled || isReadOnly - }, - inputProps: mergeProps(inputProps, { - role: 'combobox', - 'aria-expanded': menuTriggerProps['aria-expanded'], - 'aria-controls': state.isOpen ? menuProps.id : undefined, - // TODO: readd proper logic for completionMode = complete (aria-autocomplete: both) - 'aria-autocomplete': 'list', - 'aria-activedescendant': focusedItem ? getItemId(state, focusedItem.key) : undefined, - onTouchEnd, - // This disable's iOS's autocorrect suggestions, since the combo box provides its own suggestions. - autoCorrect: 'off', - // This disable's the macOS Safari spell check auto corrections. - spellCheck: 'false' - }), - listBoxProps: mergeProps(menuProps, listBoxProps, { - autoFocus: state.focusStrategy, - shouldUseVirtualFocus: true, - shouldSelectOnPressUp: true, - shouldFocusOnHover: true, - linkBehavior: 'selection' as const - }), - descriptionProps, - errorMessageProps, - isInvalid, - validationErrors, - validationDetails - }; -} diff --git a/packages/@react-spectrum/s2/package.json b/packages/@react-spectrum/s2/package.json index c2359c75f30..a00def604d1 100644 --- a/packages/@react-spectrum/s2/package.json +++ b/packages/@react-spectrum/s2/package.json @@ -132,6 +132,7 @@ "@react-aria/utils": "^3.26.0", "@react-spectrum/utils": "^3.12.0", "@react-stately/layout": "^4.1.0", + "@react-stately/menu": "^3.9.0", "@react-stately/utils": "^3.10.5", "@react-stately/virtualizer": "^4.2.0", "@react-types/color": "^3.0.1", @@ -143,8 +144,7 @@ "@react-types/textfield": "^3.10.0", "csstype": "^3.0.2", "react-aria": "^3.36.0", - "react-aria-components": "^1.5.0", - "react-stately": "^3.34.0" + "react-aria-components": "^1.5.0" }, "peerDependencies": { "@testing-library/react": "^15.0.7", diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index 1eccb0eddf3..2e16d0628b8 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -39,7 +39,7 @@ import {PressResponder} from '@react-aria/interactions'; import {SliderContext} from './Slider'; import {space, style} from '../style' with {type: 'macro'}; import {useId, useOverlayTrigger} from 'react-aria'; -import {useMenuTriggerState} from 'react-stately'; +import {useMenuTriggerState} from '@react-stately/menu'; export interface CoachMarkProps extends Omit { /** The children of the coach mark. */ diff --git a/packages/react-aria-components/src/CoachMark.tsx b/packages/react-aria-components/src/CoachMark.tsx deleted file mode 100644 index ad2fb895736..00000000000 --- a/packages/react-aria-components/src/CoachMark.tsx +++ /dev/null @@ -1,18 +0,0 @@ - -export function CoachMark(props) { - return ( -
- ); -} - -export function CoachIndicator(props) { - return ( -
- ); -} - -export function Tour(props) { - return ( -
- ); -} diff --git a/packages/react-aria-components/src/SearchMenu.tsx b/packages/react-aria-components/src/SearchMenu.tsx deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/packages/react-aria-components/stories/SearchMenu.stories.tsx b/packages/react-aria-components/stories/SearchMenu.stories.tsx deleted file mode 100644 index 412ba28435e..00000000000 --- a/packages/react-aria-components/stories/SearchMenu.stories.tsx +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright 2022 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ - -import {Button, Input, Label, SearchField} from 'react-aria-components'; -import {classNames} from '@react-spectrum/utils'; -import React from 'react'; -import styles from '../example/index.css'; - -export default { - title: 'React Aria Components' -}; - -export const SearchFMenuExample = () => { - return ( - - - - - - ); -}; diff --git a/yarn.lock b/yarn.lock index 7f47e45bd49..7fe6e3bf153 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7861,6 +7861,7 @@ __metadata: "@react-aria/utils": "npm:^3.26.0" "@react-spectrum/utils": "npm:^3.12.0" "@react-stately/layout": "npm:^4.1.0" + "@react-stately/menu": "npm:^3.9.0" "@react-stately/utils": "npm:^3.10.5" "@react-stately/virtualizer": "npm:^4.2.0" "@react-types/color": "npm:^3.0.1" From a1043097bf3a8de940fca7fa95fad7e0cd2ded04 Mon Sep 17 00:00:00 2001 From: Jeff Luyau Date: Thu, 9 Jan 2025 11:15:41 -0800 Subject: [PATCH 04/23] remove unneeded import --- packages/react-aria-components/src/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-aria-components/src/index.ts b/packages/react-aria-components/src/index.ts index 50b093a07ba..1950d15570c 100644 --- a/packages/react-aria-components/src/index.ts +++ b/packages/react-aria-components/src/index.ts @@ -21,7 +21,6 @@ export {Breadcrumbs, BreadcrumbsContext, Breadcrumb} from './Breadcrumbs'; export {Button, ButtonContext} from './Button'; export {Calendar, CalendarGrid, CalendarGridHeader, CalendarGridBody, CalendarHeaderCell, CalendarCell, RangeCalendar, CalendarContext, RangeCalendarContext, CalendarStateContext, RangeCalendarStateContext} from './Calendar'; export {Checkbox, CheckboxGroup, CheckboxGroupContext, CheckboxGroupStateContext} from './Checkbox'; -export {CoachMark, CoachIndicator, Tour} from './CoachMark'; export {ColorArea, ColorAreaStateContext} from './ColorArea'; export {ColorField, ColorFieldStateContext} from './ColorField'; export {ColorPicker, ColorPickerContext, ColorPickerStateContext} from './ColorPicker'; From 1162a44752e35ba3919f90c02ffbaf7cf5daf940 Mon Sep 17 00:00:00 2001 From: Jeff Luyau Date: Thu, 9 Jan 2025 11:35:47 -0800 Subject: [PATCH 05/23] lint fix --- packages/@react-spectrum/s2/stories/CoachMark.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index 7bd4bdbcf5b..46c248938f5 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -19,7 +19,7 @@ import { CoachMark, CoachMarkTrigger, Content, Footer, Image, Keyboard, MenuItem, - Slider, StatusLight, + Slider, Text, useTour } from '../src'; import Filter from '../s2wf-icons/S2_Icon_Filter_20_N.svg'; From 8cbcadf71931b6fe77b2966fc1299e46148dd262 Mon Sep 17 00:00:00 2001 From: Jeff Luyau Date: Thu, 9 Jan 2025 11:50:29 -0800 Subject: [PATCH 06/23] more lint fixes --- packages/@react-spectrum/s2/src/CoachMark.tsx | 2 +- packages/@react-spectrum/s2/stories/CoachMark.stories.tsx | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index 2e16d0628b8..dab7106f2fa 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -45,7 +45,7 @@ export interface CoachMarkProps extends Omit { /** The children of the coach mark. */ children: ReactNode | ((renderProps: { size: 'XS' | 'S' | 'M' | 'L' | 'XL' }) => ReactNode), - size: 'S' | 'M' | 'L' | 'XL' + size?: 'S' | 'M' | 'L' | 'XL' } const fadeKeyframes = keyframes(` diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index 46c248938f5..7db4c59a0bb 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -24,6 +24,7 @@ import { } from '../src'; import Filter from '../s2wf-icons/S2_Icon_Filter_20_N.svg'; import type {Meta} from '@storybook/react'; +import {Placement} from '@react-types/overlays'; import {style} from '../style' with {type: 'macro'}; import TextAdd from '../s2wf-icons/S2_Icon_TextAdd_20_N.svg'; import TextBold from '../s2wf-icons/S2_Icon_TextBold_20_N.svg'; @@ -72,7 +73,7 @@ export const CoachMarkExample = (args) => ( ); -function CoachMarkBase({step, currentStep, totalSteps, description = '', skipTour, restartTour, advanceStep, previousStep, hasPressAction = false, placement = 'right top'}) { +function CoachMarkBase({step, currentStep, totalSteps, description = '', skipTour, restartTour, advanceStep, previousStep, hasPressAction = false, placement = 'right top' as Placement}) { const onAction = actionKey => { if (actionKey === 'skip') { skipTour(); From a33a53389a5b6cd4398f0b6647ff001ba24a22ef Mon Sep 17 00:00:00 2001 From: Jeff Luyau Date: Thu, 9 Jan 2025 12:09:38 -0800 Subject: [PATCH 07/23] remove old stories --- .../s2/stories/CoachMark.stories.tsx | 40 ------------------- 1 file changed, 40 deletions(-) diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index 7db4c59a0bb..e4ea4730eb7 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -161,43 +161,3 @@ export const TourExample = () => {
); }; - -// export const Test = () => ( -// ); -// -// export const Indicator = (args: any) => (
); -export const IndicatorButton = (args: any) => ( - -); - -export const IndicatorActionButton = (args: any) => ( - - - -); - -export const IndicatorToolbar = (args: any) => ( - - - - - - - - - - - -); - -export const IndicatorSlider = (args: any) => ( - -); - -export const IndicatorCheckbox = (args: any) => ( - Sync with CC -); - -// export const LongLabel = (args: any) => (Checkbox with very long label so we can see wrapping); From 6c01220f50d9d5c16ff8d5aad9a3d81ac9f04959 Mon Sep 17 00:00:00 2001 From: Jeff Luyau Date: Thu, 9 Jan 2025 12:24:07 -0800 Subject: [PATCH 08/23] remove unused import --- packages/@react-spectrum/s2/stories/CoachMark.stories.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index e4ea4730eb7..44850f5ac67 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -15,7 +15,6 @@ import { Button, CardPreview, Checkbox, - CoachIndicator, CoachMark, CoachMarkTrigger, Content, Footer, Image, Keyboard, MenuItem, From 3601365e9aa1fc2f847026e8c27c9ebd2c464ab7 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 11 Mar 2025 11:11:44 +1100 Subject: [PATCH 09/23] fix yarn lock --- yarn.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/yarn.lock b/yarn.lock index 9a608dec0c2..fe3e0b6e551 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7889,6 +7889,7 @@ __metadata: "@react-aria/utils": "npm:^3.28.1" "@react-spectrum/utils": "npm:^3.12.3" "@react-stately/layout": "npm:^4.2.1" + "@react-stately/menu": "npm:^3.9.2" "@react-stately/utils": "npm:^3.10.5" "@react-types/dialog": "npm:^3.5.16" "@react-types/grid": "npm:^3.3.0" From 7fb0ea7506b0b0b35ecb28b04db3254203ecdcff Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 20 Mar 2025 15:48:51 +1100 Subject: [PATCH 10/23] API changes to bring in line with other s2 components --- packages/@react-spectrum/s2/package.json | 1 + packages/@react-spectrum/s2/src/CoachMark.tsx | 92 ++++++++++----- .../s2/stories/CoachMark.stories.tsx | 110 +++++++++--------- .../s2/test/CoachMark.test.tsx | 66 +++++++++++ 4 files changed, 184 insertions(+), 85 deletions(-) create mode 100644 packages/@react-spectrum/s2/test/CoachMark.test.tsx diff --git a/packages/@react-spectrum/s2/package.json b/packages/@react-spectrum/s2/package.json index a62434cd588..db137693363 100644 --- a/packages/@react-spectrum/s2/package.json +++ b/packages/@react-spectrum/s2/package.json @@ -134,6 +134,7 @@ "@react-aria/i18n": "^3.12.7", "@react-aria/interactions": "^3.24.1", "@react-aria/live-announcer": "^3.4.1", + "@react-aria/overlays": "^3.26.1", "@react-aria/utils": "^3.28.1", "@react-spectrum/utils": "^3.12.3", "@react-stately/layout": "^4.2.1", diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index dab7106f2fa..602d468ce6b 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -14,22 +14,24 @@ import {ActionMenuContext} from './ActionMenu'; import { DialogTriggerProps as AriaDialogTriggerProps, Popover as AriaPopover, + ContextValue, DEFAULT_SLOT, DialogContext, OverlayTriggerStateContext, PopoverContext, PopoverProps, Provider, - RootMenuTriggerStateContext + RootMenuTriggerStateContext, + useContextProps } from 'react-aria-components'; import {ButtonContext} from './Button'; import {Card} from './Card'; import {CheckboxContext} from './Checkbox'; -import {colorScheme, getAllowedOverrides} from './style-utils' with {type: 'macro'}; +import {colorScheme, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import {ColorSchemeContext} from './Provider'; import {ContentContext, FooterContext, KeyboardContext, TextContext} from './Content'; +import {createContext, ForwardedRef, forwardRef, ReactNode, useContext, useRef, useState} from 'react'; import {DividerContext} from './Divider'; -import {ForwardedRef, forwardRef, ReactNode, useContext, useRef, useState} from 'react'; import {forwardRefType} from './types'; import {ImageContext} from './Image'; import {ImageCoordinator} from './ImageCoordinator'; @@ -40,10 +42,11 @@ import {SliderContext} from './Slider'; import {space, style} from '../style' with {type: 'macro'}; import {useId, useOverlayTrigger} from 'react-aria'; import {useMenuTriggerState} from '@react-stately/menu'; +import {useObjectRef} from '@react-aria/utils'; -export interface CoachMarkProps extends Omit { +export interface CoachMarkProps extends Omit, StyleProps { /** The children of the coach mark. */ - children: ReactNode | ((renderProps: { size: 'XS' | 'S' | 'M' | 'L' | 'XL' }) => ReactNode), + children: ReactNode, size?: 'S' | 'M' | 'L' | 'XL' } @@ -310,9 +313,17 @@ const actionButtonSize = { XL: 'L' } as const; -export function CoachMark(props: CoachMarkProps) { +export const CoachMarkContext = createContext>({}); + +// there is a focus trap outside of the Coachmark, can only forward tab to things in coachmark or after the trigger +export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef) => { let colorScheme = useContext(ColorSchemeContext); + [props, ref] = useContextProps(props, ref, CoachMarkContext); + // let {isOpen} = useContext(OverlayTriggerStateContext)!; // valid assumption? + let {UNSAFE_style} = props; + // let {triggerRef} = useContext(PopoverContext); let {size = 'M'} = props; + let popoverRef = useObjectRef(ref); let children = ( - {typeof props.children === 'function' ? props.children({size}) : props.children} + {props.children} ); + + // useLayoutEffect(() => { // do we want to hide everything so that it's easier for a AT user to get to the trigger for required actions? + // if (isOpen) { + // let cleanup = ariaHideOutside([triggerRef.current, popoverRef.current]); + // return () => { + // cleanup(); + // }; + // } + // }, [isOpen]); + return ( ); -} +}); export interface CoachMarkTriggerProps extends AriaDialogTriggerProps { @@ -372,12 +393,12 @@ export interface CoachMarkTriggerProps extends AriaDialogTriggerProps { * open state with the trigger's press state. Additionally, it allows you to customize the type and * positioning of the Dialog. */ -export function CoachMarkTrigger(props: CoachMarkTriggerProps) { +export function CoachMarkTrigger(props: CoachMarkTriggerProps): ReactNode { + let containerRef = useRef(null); // Use useMenuTriggerState instead of useOverlayTriggerState in case a menu is embedded in the dialog. // This is needed to handle submenus. let state = useMenuTriggerState(props); - let containerRef = useRef(null); let {triggerProps, overlayProps} = useOverlayTrigger({type: 'dialog'}, state, containerRef); // Label dialog by the trigger as a fallback if there is no title slot. @@ -394,9 +415,9 @@ export function CoachMarkTrigger(props: CoachMarkTriggerProps) { [OverlayTriggerStateContext, state], [RootMenuTriggerStateContext, state], [DialogContext, overlayProps], - [PopoverContext, {trigger: 'DialogTrigger', triggerRef: containerRef, isNonModal: true}] + [PopoverContext, {trigger: 'DialogTrigger', triggerRef: containerRef, isNonModal: true}] // valid to pass triggerRef? ]}> - + {props.children} @@ -404,19 +425,6 @@ export function CoachMarkTrigger(props: CoachMarkTriggerProps) { ); } -// export function CoachMarkTrigger(props: CoachMarkTriggerProps) { -// return ( -// -// {/* RAC sets isPressed via PressResponder when the dialog is open. -// We don't want press scaling to appear to get "stuck", so override this. */} -// -// -// {props.children} -// -// -// -// ); -// } // TODO better way to calculate 4px transform? (not 4%?) @@ -504,11 +512,31 @@ export const CoachIndicator = /*#__PURE__*/ (forwardRef as forwardRefType)(funct ); }); +export interface TourProps { + defaultStep?: number, + defaultComplete?: boolean, + /** + * Used to pre-set the total number of steps when more coachmarks may arrive async. + */ + totalSteps?: number +} + +export interface TourReturnVal { + createCoachMarkTriggerProps: (step: number) => Omit, + currentStep: number, + totalSteps: number, + advanceStep: () => void, + previousStep: () => void, + skipTour: () => void, + restartTour: () => void +} + // FUTURE: support multiple tours on a page? -export function useTour({defaultStep = 1, defaultComplete = false}) { +export function useTour(props: TourProps): TourReturnVal { + let {defaultStep = 1, defaultComplete = false, totalSteps: controlledTotalSteps} = props; const [isComplete, setIsComplete] = useState(defaultComplete); const [currentStep, setCurrentStep] = useState(defaultStep); - const [totalSteps, setTotalSteps] = useState(0); + const [totalSteps, setTotalSteps] = useState(controlledTotalSteps ?? 0); const advanceStep = () => { if (currentStep + 1 > totalSteps) { setIsComplete(true); @@ -522,9 +550,9 @@ export function useTour({defaultStep = 1, defaultComplete = false}) { setCurrentStep(1); }; - const coachMarkTriggerProps = (step: number) => { + const createCoachMarkTriggerProps = (step: number): Omit => { // TODO find a better way for this so it's not run on every rerender - if (step > totalSteps) { + if (step > totalSteps && controlledTotalSteps === undefined) { setTotalSteps(step); } return { @@ -533,7 +561,7 @@ export function useTour({defaultStep = 1, defaultComplete = false}) { }; return { - coachMarkTriggerProps, + createCoachMarkTriggerProps, currentStep, totalSteps, advanceStep, diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index 44850f5ac67..dd77e44895a 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -48,7 +48,7 @@ const meta: Meta = { export default meta; export const CoachMarkExample = (args) => ( - + Sync with CC @@ -72,7 +72,7 @@ export const CoachMarkExample = (args) => ( ); -function CoachMarkBase({step, currentStep, totalSteps, description = '', skipTour, restartTour, advanceStep, previousStep, hasPressAction = false, placement = 'right top' as Placement}) { +function MyCoachMark({step, currentStep, totalSteps, description = '', skipTour, restartTour, advanceStep, previousStep, hasPressAction = false, placement = 'right top' as Placement}) { const onAction = actionKey => { if (actionKey === 'skip') { skipTour(); @@ -97,7 +97,7 @@ function CoachMarkBase({step, currentStep, totalSteps, description = '', skipTou
{currentStep} of {totalSteps} {step !== 1 && } - {step !== totalSteps && !hasPressAction && } + {step !== totalSteps && !hasPressAction && } {step === totalSteps && !hasPressAction && }
@@ -105,58 +105,62 @@ function CoachMarkBase({step, currentStep, totalSteps, description = '', skipTou } export const TourExample = () => { - let {coachMarkTriggerProps, ...otherTourProps} = useTour({}); - return (
-
- - - - - - + let {createCoachMarkTriggerProps, ...otherTourProps} = useTour({defaultComplete: true}); + return ( +
+
+ + + + + + - - - - + + + + - - Sync with CC - - -
-
-
- - - - + + Sync with CC + + +
+
+ +
+
+ + + + - - - - - - - - - - - - - - + + + + + + + + + + + + + + +
-
); + ); }; diff --git a/packages/@react-spectrum/s2/test/CoachMark.test.tsx b/packages/@react-spectrum/s2/test/CoachMark.test.tsx new file mode 100644 index 00000000000..6caeec91271 --- /dev/null +++ b/packages/@react-spectrum/s2/test/CoachMark.test.tsx @@ -0,0 +1,66 @@ +/* + * Copyright 2022 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {act, pointerMap, render} from '@react-spectrum/test-utils-internal'; +import {ActionMenu, Button, CardPreview, Checkbox, CoachMark, CoachMarkTrigger, Content, Footer, Image, Keyboard, MenuItem, Text} from '../src'; +import React from 'react'; +import userEvent, {UserEvent} from '@testing-library/user-event'; + +const mockAnimations = () => { + Element.prototype.animate = jest.fn().mockImplementation(() => ({finished: Promise.resolve()})); +}; + +describe('CoachMark', () => { + let user: UserEvent | null = null; + beforeAll(() => { + jest.useFakeTimers(); + mockAnimations(); + }); + beforeEach(() => { + user = userEvent.setup({delay: null, pointerMap}); + }); + afterAll(() => { + act(() => {jest.runAllTimers();}); + }); + + it('renders a coachmark', async () => { + let onPress = jest.fn(); + let {getAllByRole} = render( + + Sync with CC + + + + + + Hello + + Skip tour + Restart tour + + Command + B + This is the description + +
+ 1 of 10 + + +
+
+
+ ); + act(() => {jest.runAllTimers();}); + expect(getAllByRole('button').length).toBe(4); // 2 Dismiss + 2 actions + await user?.click(getAllByRole('button')[2]); + expect(onPress).toHaveBeenCalled(); + }); +}); From be9343b95e6be1e7badf5ddfef355689bb202ff4 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 10 Apr 2025 11:31:32 +1000 Subject: [PATCH 11/23] Cleanup to get merged --- packages/@react-spectrum/s2/src/CoachMark.tsx | 115 +++---- packages/@react-spectrum/s2/src/index.ts | 2 +- .../s2/stories/CoachMark.stories.tsx | 280 ++++++++++-------- yarn.lock | 1 + 4 files changed, 193 insertions(+), 205 deletions(-) diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index 602d468ce6b..d863f50a1c7 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -30,7 +30,7 @@ import {CheckboxContext} from './Checkbox'; import {colorScheme, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import {ColorSchemeContext} from './Provider'; import {ContentContext, FooterContext, KeyboardContext, TextContext} from './Content'; -import {createContext, ForwardedRef, forwardRef, ReactNode, useContext, useRef, useState} from 'react'; +import {createContext, ForwardedRef, forwardRef, ReactNode, useContext, useLayoutEffect, useRef, useState} from 'react'; import {DividerContext} from './Divider'; import {forwardRefType} from './types'; import {ImageContext} from './Image'; @@ -44,7 +44,7 @@ import {useId, useOverlayTrigger} from 'react-aria'; import {useMenuTriggerState} from '@react-stately/menu'; import {useObjectRef} from '@react-aria/utils'; -export interface CoachMarkProps extends Omit, StyleProps { +export interface CoachMarkProps extends Omit, StyleProps { /** The children of the coach mark. */ children: ReactNode, @@ -315,13 +315,10 @@ const actionButtonSize = { export const CoachMarkContext = createContext>({}); -// there is a focus trap outside of the Coachmark, can only forward tab to things in coachmark or after the trigger export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef) => { let colorScheme = useContext(ColorSchemeContext); [props, ref] = useContextProps(props, ref, CoachMarkContext); - // let {isOpen} = useContext(OverlayTriggerStateContext)!; // valid assumption? let {UNSAFE_style} = props; - // let {triggerRef} = useContext(PopoverContext); let {size = 'M'} = props; let popoverRef = useObjectRef(ref); @@ -355,15 +352,6 @@ export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef ); - // useLayoutEffect(() => { // do we want to hide everything so that it's easier for a AT user to get to the trigger for required actions? - // if (isOpen) { - // let cleanup = ariaHideOutside([triggerRef.current, popoverRef.current]); - // return () => { - // cleanup(); - // }; - // } - // }, [isOpen]); - return ( (null); + let triggerRef = useRef(null); // Use useMenuTriggerState instead of useOverlayTriggerState in case a menu is embedded in the dialog. // This is needed to handle submenus. let state = useMenuTriggerState(props); - let {triggerProps, overlayProps} = useOverlayTrigger({type: 'dialog'}, state, containerRef); + let {triggerProps, overlayProps} = useOverlayTrigger({type: 'dialog'}, state, triggerRef); // Label dialog by the trigger as a fallback if there is no title slot. // This is done in RAC instead of hooks because otherwise we cannot distinguish @@ -415,12 +403,12 @@ export function CoachMarkTrigger(props: CoachMarkTriggerProps): ReactNode { [OverlayTriggerStateContext, state], [RootMenuTriggerStateContext, state], [DialogContext, overlayProps], - [PopoverContext, {trigger: 'DialogTrigger', triggerRef: containerRef, isNonModal: true}] // valid to pass triggerRef? + [PopoverContext, {trigger: 'DialogTrigger', triggerRef, isNonModal: true}] // valid to pass triggerRef? ]}> - + {props.children} - + ); @@ -481,16 +469,38 @@ const indicator = style({ const pulse = raw(`&:before { content: ""; display: inline-block; position: absolute; top: var(--borderOffset); bottom: var(--borderOffset); left: var(--borderOffset); right: var(--borderOffset); border-radius: var(--ringRadius); outline-style: solid; outline-color: var(--activeElement); outline-width: 4px; animation-duration: 2s; animation-iteration-count: infinite; animation-timing-function: ease-in-out; animation-fill-mode: forwards; animation-name: ${pulseAnimation}}`); -interface CoachIndicatorProps { +interface CoachMarkIndicatorProps { children: ReactNode, isActive?: boolean } -export const CoachIndicator = /*#__PURE__*/ (forwardRef as forwardRefType)(function CoachIndicator(props: CoachIndicatorProps, ref: ForwardedRef) { -// export const CoachIndicator = forwardRef((props, ref) => { +export const CoachMarkIndicator = /*#__PURE__*/ (forwardRef as forwardRefType)(function CoachMarkIndicator(props: CoachMarkIndicatorProps, ref: ForwardedRef) { const {children, isActive} = props; + let objRef = useObjectRef(ref); + + // This is very silly... better ways? can't use display: contents because it breaks positioning + // this will break if there is a resize or different styles + useLayoutEffect(() => { + if (objRef.current) { + let styles = getComputedStyle(objRef.current.children[0]); + let childDisplay = styles.getPropertyValue('display'); + let childMaxWidth = styles.getPropertyValue('max-width'); + let childMaxHeight = styles.getPropertyValue('max-height'); + let childWidth = styles.getPropertyValue('width'); + let childHeight = styles.getPropertyValue('height'); + let childMinWidth = styles.getPropertyValue('min-width'); + let childMinHeight = styles.getPropertyValue('min-height'); + objRef.current.style.display = childDisplay; + objRef.current.style.maxWidth = childMaxWidth; + objRef.current.style.maxHeight = childMaxHeight; + objRef.current.style.width = childWidth; + objRef.current.style.height = childHeight; + objRef.current.style.minWidth = childMinWidth; + objRef.current.style.minHeight = childMinHeight; + } + }, [children]); return ( -
+
); }); - -export interface TourProps { - defaultStep?: number, - defaultComplete?: boolean, - /** - * Used to pre-set the total number of steps when more coachmarks may arrive async. - */ - totalSteps?: number -} - -export interface TourReturnVal { - createCoachMarkTriggerProps: (step: number) => Omit, - currentStep: number, - totalSteps: number, - advanceStep: () => void, - previousStep: () => void, - skipTour: () => void, - restartTour: () => void -} - -// FUTURE: support multiple tours on a page? -export function useTour(props: TourProps): TourReturnVal { - let {defaultStep = 1, defaultComplete = false, totalSteps: controlledTotalSteps} = props; - const [isComplete, setIsComplete] = useState(defaultComplete); - const [currentStep, setCurrentStep] = useState(defaultStep); - const [totalSteps, setTotalSteps] = useState(controlledTotalSteps ?? 0); - const advanceStep = () => { - if (currentStep + 1 > totalSteps) { - setIsComplete(true); - } - setCurrentStep(currentStep < totalSteps ? currentStep + 1 : totalSteps); - }; - const previousStep = () => setCurrentStep(currentStep > 0 ? currentStep - 1 : 0); - const skipTour = () => setIsComplete(true); - const restartTour = () => { - setIsComplete(false); - setCurrentStep(1); - }; - - const createCoachMarkTriggerProps = (step: number): Omit => { - // TODO find a better way for this so it's not run on every rerender - if (step > totalSteps && controlledTotalSteps === undefined) { - setTotalSteps(step); - } - return { - isOpen: !isComplete && step === currentStep - }; - }; - - return { - createCoachMarkTriggerProps, - currentStep, - totalSteps, - advanceStep, - previousStep, - skipTour, - restartTour - }; -} diff --git a/packages/@react-spectrum/s2/src/index.ts b/packages/@react-spectrum/s2/src/index.ts index c5f883f5185..0609204faaa 100644 --- a/packages/@react-spectrum/s2/src/index.ts +++ b/packages/@react-spectrum/s2/src/index.ts @@ -27,7 +27,7 @@ export {CardView, CardViewContext} from './CardView'; export {Checkbox, CheckboxContext} from './Checkbox'; export {CheckboxGroup, CheckboxGroupContext} from './CheckboxGroup'; export {CloseButton} from './CloseButton'; -export {CoachMark, CoachMarkTrigger, CoachIndicator, useTour} from './CoachMark'; +export {CoachMark as UNSTABLE_CoachMark, CoachMarkTrigger as UNSTABLE_CoachMarkTrigger} from './CoachMark'; export {ColorArea, ColorAreaContext} from './ColorArea'; export {ColorField, ColorFieldContext} from './ColorField'; export {ColorSlider, ColorSliderContext} from './ColorSlider'; diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index dd77e44895a..063fc5e4190 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -9,33 +9,32 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ - import { - ActionButton, ActionMenu, + ActionButton, + ActionMenu, Button, CardPreview, Checkbox, - CoachMark, - CoachMarkTrigger, - Content, Footer, Image, Keyboard, MenuItem, + UNSTABLE_CoachMark as CoachMark, + UNSTABLE_CoachMarkTrigger as CoachMarkTrigger, + Content, + Footer, + Image, + Keyboard, + MenuItem, Slider, - Text, useTour + Text } from '../src'; import Filter from '../s2wf-icons/S2_Icon_Filter_20_N.svg'; -import type {Meta} from '@storybook/react'; -import {Placement} from '@react-types/overlays'; +import type {Meta, StoryObj} from '@storybook/react'; import {style} from '../style' with {type: 'macro'}; -import TextAdd from '../s2wf-icons/S2_Icon_TextAdd_20_N.svg'; -import TextBold from '../s2wf-icons/S2_Icon_TextBold_20_N.svg'; -import TextIcon from '../s2wf-icons/S2_Icon_Text_20_N.svg'; -import {Toolbar} from 'react-aria-components'; +import {useState} from 'react'; const meta: Meta = { component: CoachMark, parameters: { layout: 'centered' }, - tags: ['autodocs'], argTypes: { placement: { control: 'radio', @@ -46,121 +45,158 @@ const meta: Meta = { }; export default meta; +type Story = StoryObj; -export const CoachMarkExample = (args) => ( - - Sync with CC - - - - - - Hello - - Skip tour - Restart tour - - Command + B - This is the description - -
- 1 of 10 - - -
-
-
-); - -function MyCoachMark({step, currentStep, totalSteps, description = '', skipTour, restartTour, advanceStep, previousStep, hasPressAction = false, placement = 'right top' as Placement}) { - const onAction = actionKey => { - if (actionKey === 'skip') { - skipTour(); - } else if (actionKey === 'restart') { - restartTour(); +export const CoachMarkExample: Story = { + render: (args) => ( +
+ + + Sync with CC + + + + + + Hello + + Skip tour + Restart tour + + Command + B + This is the description + +
+ 1 of 10 + + +
+
+
+ +
+ ), + parameters: { + docs: { + disable: true } - }; + } +}; + +function ControlledCoachMark(args) { + let [isOpen, setIsOpen] = useState(false); + return ( - - - - - - Coach mark title - - Skip tour - {step !== 1 && Restart tour} - - Command + B - {description || 'This is the description'} - -
- {currentStep} of {totalSteps} - {step !== 1 && } - {step !== totalSteps && !hasPressAction && } - {step === totalSteps && !hasPressAction && } -
-
+
+ + + Sync with CC + + + + + + Hello + + Skip tour + Restart tour + + Command + B + This is the description + +
+ 1 of 10 + + +
+
+
+ +
); } -export const TourExample = () => { - let {createCoachMarkTriggerProps, ...otherTourProps} = useTour({defaultComplete: true}); - return ( -
-
- - - - - - - - - - - - - - Sync with CC - - -
-
- -
-
- - - - +export const CoachMarkRestartable: Story = { + render: (args) => ( + + ), + parameters: { + docs: { + disable: true + } + } +}; - - - - - - - - - - - - - - -
+export const CoachMarkSlider: Story = { + render: (args) => ( +
+ + + + + + + + + Hello + + Skip tour + Restart tour + + Command + B + This is the description + +
+ 1 of 10 + + +
+
+
+
- ); + ), + parameters: { + docs: { + disable: true + } + } }; + +export const CoachMarkButton: Story = { + render: (args) => ( +
+ + + + + + + + + + + Hello + + Skip tour + Restart tour + + Command + B + This is the description + +
+ 1 of 10 + + +
+
+
+ +
+ ), + parameters: { + docs: { + disable: true + } + } +}; \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index 54a310d4e1f..1ce864c43cd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7941,6 +7941,7 @@ __metadata: "@react-aria/i18n": "npm:^3.12.7" "@react-aria/interactions": "npm:^3.24.1" "@react-aria/live-announcer": "npm:^3.4.1" + "@react-aria/overlays": "npm:^3.26.1" "@react-aria/test-utils": "npm:1.0.0-alpha.3" "@react-aria/utils": "npm:^3.28.1" "@react-spectrum/utils": "npm:^3.12.3" From 7f0f3086d970abeb032a1217b8bf767da2e8f599 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 10 Apr 2025 11:52:59 +1000 Subject: [PATCH 12/23] fix lint and tests --- packages/@react-spectrum/s2/src/CoachMark.tsx | 13 ++++++++++--- .../s2/stories/CoachMark.stories.tsx | 2 +- .../@react-spectrum/s2/test/CoachMark.test.tsx | 15 ++++++++++++++- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index d863f50a1c7..4edb0a55c84 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -30,7 +30,14 @@ import {CheckboxContext} from './Checkbox'; import {colorScheme, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import {ColorSchemeContext} from './Provider'; import {ContentContext, FooterContext, KeyboardContext, TextContext} from './Content'; -import {createContext, ForwardedRef, forwardRef, ReactNode, useContext, useLayoutEffect, useRef, useState} from 'react'; +import { + createContext, + ForwardedRef, + forwardRef, + ReactNode, + useContext, + useRef +} from 'react'; import {DividerContext} from './Divider'; import {forwardRefType} from './types'; import {ImageContext} from './Image'; @@ -40,9 +47,9 @@ import {mergeStyles} from '../style/runtime'; import {PressResponder} from '@react-aria/interactions'; import {SliderContext} from './Slider'; import {space, style} from '../style' with {type: 'macro'}; -import {useId, useOverlayTrigger} from 'react-aria'; +import {useId, useObjectRef, useOverlayTrigger} from 'react-aria'; +import {useLayoutEffect} from '@react-aria/utils'; import {useMenuTriggerState} from '@react-stately/menu'; -import {useObjectRef} from '@react-aria/utils'; export interface CoachMarkProps extends Omit, StyleProps { /** The children of the coach mark. */ diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index 063fc5e4190..8702569cf6c 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -199,4 +199,4 @@ export const CoachMarkButton: Story = { disable: true } } -}; \ No newline at end of file +}; diff --git a/packages/@react-spectrum/s2/test/CoachMark.test.tsx b/packages/@react-spectrum/s2/test/CoachMark.test.tsx index 6caeec91271..a413efba824 100644 --- a/packages/@react-spectrum/s2/test/CoachMark.test.tsx +++ b/packages/@react-spectrum/s2/test/CoachMark.test.tsx @@ -11,7 +11,20 @@ */ import {act, pointerMap, render} from '@react-spectrum/test-utils-internal'; -import {ActionMenu, Button, CardPreview, Checkbox, CoachMark, CoachMarkTrigger, Content, Footer, Image, Keyboard, MenuItem, Text} from '../src'; +import { + ActionMenu, + Button, + CardPreview, + Checkbox, + UNSTABLE_CoachMark as CoachMark, + UNSTABLE_CoachMarkTrigger as CoachMarkTrigger, + Content, + Footer, + Image, + Keyboard, + MenuItem, + Text +} from '../src'; import React from 'react'; import userEvent, {UserEvent} from '@testing-library/user-event'; From 6cc2a372f83820f1db6a30d1e0074ff9acd7f193 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Mon, 14 Apr 2025 13:25:09 +1000 Subject: [PATCH 13/23] feat: CoachMark native popover --- packages/@react-spectrum/s2/src/Card.tsx | 2 +- .../s2/src/CoachMark.module.css | 45 ++++ packages/@react-spectrum/s2/src/CoachMark.tsx | 241 +++++++----------- .../s2/stories/CoachMark.stories.tsx | 51 ++-- .../s2/style/spectrum-theme.ts | 1 + 5 files changed, 162 insertions(+), 178 deletions(-) create mode 100644 packages/@react-spectrum/s2/src/CoachMark.module.css diff --git a/packages/@react-spectrum/s2/src/Card.tsx b/packages/@react-spectrum/s2/src/Card.tsx index 751388b4b2a..c289b8dc614 100644 --- a/packages/@react-spectrum/s2/src/Card.tsx +++ b/packages/@react-spectrum/s2/src/Card.tsx @@ -65,7 +65,7 @@ const borderRadius = { } } as const; -let card = style({ +export const card = style({ display: 'flex', flexDirection: 'column', position: 'relative', diff --git a/packages/@react-spectrum/s2/src/CoachMark.module.css b/packages/@react-spectrum/s2/src/CoachMark.module.css new file mode 100644 index 00000000000..98e4b20dde1 --- /dev/null +++ b/packages/@react-spectrum/s2/src/CoachMark.module.css @@ -0,0 +1,45 @@ +/* spectrum theme doesn't support starting style yet, so use css modules. Also, support for transition behavior isn't implemented yet. */ +.coach-mark { + /* must have overlay transition to prevent layout shift when closing */ + transition: display allow-discrete 200ms, overlay allow-discrete 200ms, opacity 200ms, transform 200ms; + will-change: opacity, transform; + opacity: 0; + + &[data-placement="bottom"] { + transform: translateX(0px) translateY(-4px); + } + &[data-placement="top"] { + transform: translateX(0px) translateY(4px); + } + &[data-placement="left"] { + transform: translateX(-4px) translateY(0px); + } + &[data-placement="right"] { + transform: translateX(4px) translateY(0px); + } + + &:popover-open { + transform: translateX(0px) translateY(0px); + opacity: 1; + } +} + +@starting-style { + .coach-mark { + &:popover-open { + &[data-placement="bottom"] { + transform: translateX(0px) translateY(-4px); + } + &[data-placement="top"] { + transform: translateX(0px) translateY(4px); + } + &[data-placement="left"] { + transform: translateX(-4px) translateY(0px); + } + &[data-placement="right"] { + transform: translateX(4px) translateY(0px); + } + opacity: 0; + } + } +} \ No newline at end of file diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index 4edb0a55c84..b9c23249268 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -25,7 +25,7 @@ import { useContextProps } from 'react-aria-components'; import {ButtonContext} from './Button'; -import {Card} from './Card'; +import {card, Card} from './Card'; import {CheckboxContext} from './Checkbox'; import {colorScheme, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import {ColorSchemeContext} from './Provider'; @@ -35,6 +35,7 @@ import { ForwardedRef, forwardRef, ReactNode, + RefObject, useContext, useRef } from 'react'; @@ -47,9 +48,10 @@ import {mergeStyles} from '../style/runtime'; import {PressResponder} from '@react-aria/interactions'; import {SliderContext} from './Slider'; import {space, style} from '../style' with {type: 'macro'}; -import {useId, useObjectRef, useOverlayTrigger} from 'react-aria'; +import {useId, useObjectRef, useOverlayPosition, useOverlayTrigger} from 'react-aria'; import {useLayoutEffect} from '@react-aria/utils'; import {useMenuTriggerState} from '@react-stately/menu'; +import coachmarkCss from './CoachMark.module.css'; export interface CoachMarkProps extends Omit, StyleProps { /** The children of the coach mark. */ @@ -58,147 +60,61 @@ export interface CoachMarkProps extends Omit>({}); +export const CoachMarkContext = createContext>({}); -export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef) => { - let colorScheme = useContext(ColorSchemeContext); +export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef) => { [props, ref] = useContextProps(props, ref, CoachMarkContext); let {UNSAFE_style} = props; let {size = 'M'} = props; let popoverRef = useObjectRef(ref); + let state = useContext(OverlayTriggerStateContext); + let {triggerRef} = useContext(InternalCoachMarkContext); let children = ( ); + let {overlayProps, placement} = useOverlayPosition({ + targetRef: triggerRef, + overlayRef: popoverRef, + placement: props.placement, + offset: 16, + crossOffset: -18 // made up + }); + + let prevOpen = useRef(state?.isOpen ?? false); + useLayoutEffect(() => { + if (state?.isOpen && !prevOpen.current) { + popoverRef.current?.showPopover(); + } else if (!state?.isOpen && prevOpen.current) { + popoverRef.current?.hidePopover(); + } + prevOpen.current = state?.isOpen ?? false; + }, [state?.isOpen]); + return ( - mergeStyles(popover({...renderProps, colorScheme}))}> - - {/* }// Reset OverlayTriggerStateContext so the buttons inside the dialog don't retain their hover state. */} - - {children} - - - + className={coachmarkCss['coach-mark'] + popover({size, placement})}> + {/* }// Reset OverlayTriggerStateContext so the buttons inside the dialog don't retain their hover state. */} + + {children} + +
); }); +const InternalCoachMarkContext = createContext<{triggerRef?: RefObject}>({}); + export interface CoachMarkTriggerProps extends AriaDialogTriggerProps { } @@ -402,21 +339,17 @@ export function CoachMarkTrigger(props: CoachMarkTriggerProps): ReactNode { // but when sent by context we want the title to win. triggerProps.id = useId(); overlayProps['aria-labelledby'] = triggerProps.id; - + delete triggerProps.onPress; return ( - - - {props.children} - - + + {props.children} + ); } @@ -484,27 +417,27 @@ export const CoachMarkIndicator = /*#__PURE__*/ (forwardRef as forwardRefType)(f const {children, isActive} = props; let objRef = useObjectRef(ref); - // This is very silly... better ways? can't use display: contents because it breaks positioning - // this will break if there is a resize or different styles - useLayoutEffect(() => { - if (objRef.current) { - let styles = getComputedStyle(objRef.current.children[0]); - let childDisplay = styles.getPropertyValue('display'); - let childMaxWidth = styles.getPropertyValue('max-width'); - let childMaxHeight = styles.getPropertyValue('max-height'); - let childWidth = styles.getPropertyValue('width'); - let childHeight = styles.getPropertyValue('height'); - let childMinWidth = styles.getPropertyValue('min-width'); - let childMinHeight = styles.getPropertyValue('min-height'); - objRef.current.style.display = childDisplay; - objRef.current.style.maxWidth = childMaxWidth; - objRef.current.style.maxHeight = childMaxHeight; - objRef.current.style.width = childWidth; - objRef.current.style.height = childHeight; - objRef.current.style.minWidth = childMinWidth; - objRef.current.style.minHeight = childMinHeight; - } - }, [children]); + // // This is very silly... better ways? can't use display: contents because it breaks positioning + // // this will break if there is a resize or different styles + // useLayoutEffect(() => { + // if (objRef.current) { + // let styles = getComputedStyle(objRef.current.children[0]); + // let childDisplay = styles.getPropertyValue('display'); + // let childMaxWidth = styles.getPropertyValue('max-width'); + // let childMaxHeight = styles.getPropertyValue('max-height'); + // let childWidth = styles.getPropertyValue('width'); + // let childHeight = styles.getPropertyValue('height'); + // let childMinWidth = styles.getPropertyValue('min-width'); + // let childMinHeight = styles.getPropertyValue('min-height'); + // objRef.current.style.display = childDisplay; + // objRef.current.style.maxWidth = childMaxWidth; + // objRef.current.style.maxHeight = childMaxHeight; + // objRef.current.style.width = childWidth; + // objRef.current.style.height = childHeight; + // objRef.current.style.minWidth = childMinWidth; + // objRef.current.style.minHeight = childMinHeight; + // } + // }, [children]); return (
diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index 8702569cf6c..eea07692ba3 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -13,6 +13,7 @@ import { ActionButton, ActionMenu, Button, + ButtonGroup, CardPreview, Checkbox, UNSTABLE_CoachMark as CoachMark, @@ -29,6 +30,7 @@ import Filter from '../s2wf-icons/S2_Icon_Filter_20_N.svg'; import type {Meta, StoryObj} from '@storybook/react'; import {style} from '../style' with {type: 'macro'}; import {useState} from 'react'; +import { card } from '../src/Card'; const meta: Meta = { component: CoachMark, @@ -68,8 +70,10 @@ export const CoachMarkExample: Story = {
1 of 10 - - + + + +
@@ -92,23 +96,20 @@ function ControlledCoachMark(args) { Sync with CC - - - - - Hello - - Skip tour - Restart tour - - Command + B - This is the description - -
- 1 of 10 - - -
+
+ + + + + Hello + + Skip tour + Restart tour + + Command + B + This is the description + +
@@ -148,8 +149,10 @@ export const CoachMarkSlider: Story = {
1 of 10 - - + + + +
@@ -186,8 +189,10 @@ export const CoachMarkButton: Story = {
1 of 10 - - + + + +
diff --git a/packages/@react-spectrum/s2/style/spectrum-theme.ts b/packages/@react-spectrum/s2/style/spectrum-theme.ts index 38ff6ce7b1a..ee2125fe721 100644 --- a/packages/@react-spectrum/s2/style/spectrum-theme.ts +++ b/packages/@react-spectrum/s2/style/spectrum-theme.ts @@ -350,6 +350,7 @@ const transitionProperty = { // var(--gp) is generated by the backgroundImage property when setting a gradient. // It includes a list of all of the custom properties used for each color stop. default: 'color, background-color, var(--gp), border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, translate, scale, rotate, filter, backdrop-filter', + display: 'display', colors: 'color, background-color, var(--gp), border-color, text-decoration-color, fill, stroke', opacity: 'opacity', shadow: 'box-shadow', From 6aad43c070bd2b2185311f23520a426ea9e484e3 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Mon, 14 Apr 2025 13:31:20 +1000 Subject: [PATCH 14/23] fix story --- packages/@react-spectrum/s2/src/CoachMark.tsx | 40 +++--- .../s2/stories/CoachMark.stories.tsx | 120 +++++++++--------- 2 files changed, 83 insertions(+), 77 deletions(-) diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index b9c23249268..34984e75dc2 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -284,7 +284,7 @@ export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef { if (state?.isOpen && !prevOpen.current) { popoverRef.current?.showPopover(); @@ -419,25 +419,25 @@ export const CoachMarkIndicator = /*#__PURE__*/ (forwardRef as forwardRefType)(f // // This is very silly... better ways? can't use display: contents because it breaks positioning // // this will break if there is a resize or different styles - // useLayoutEffect(() => { - // if (objRef.current) { - // let styles = getComputedStyle(objRef.current.children[0]); - // let childDisplay = styles.getPropertyValue('display'); - // let childMaxWidth = styles.getPropertyValue('max-width'); - // let childMaxHeight = styles.getPropertyValue('max-height'); - // let childWidth = styles.getPropertyValue('width'); - // let childHeight = styles.getPropertyValue('height'); - // let childMinWidth = styles.getPropertyValue('min-width'); - // let childMinHeight = styles.getPropertyValue('min-height'); - // objRef.current.style.display = childDisplay; - // objRef.current.style.maxWidth = childMaxWidth; - // objRef.current.style.maxHeight = childMaxHeight; - // objRef.current.style.width = childWidth; - // objRef.current.style.height = childHeight; - // objRef.current.style.minWidth = childMinWidth; - // objRef.current.style.minHeight = childMinHeight; - // } - // }, [children]); + useLayoutEffect(() => { + if (objRef.current) { + let styles = getComputedStyle(objRef.current.children[0]); + let childDisplay = styles.getPropertyValue('display'); + let childMaxWidth = styles.getPropertyValue('max-width'); + let childMaxHeight = styles.getPropertyValue('max-height'); + let childWidth = styles.getPropertyValue('width'); + let childHeight = styles.getPropertyValue('height'); + let childMinWidth = styles.getPropertyValue('min-width'); + let childMinHeight = styles.getPropertyValue('min-height'); + objRef.current.style.display = childDisplay; + objRef.current.style.maxWidth = childMaxWidth; + objRef.current.style.maxHeight = childMaxHeight; + objRef.current.style.width = childWidth; + objRef.current.style.height = childHeight; + objRef.current.style.minWidth = childMinWidth; + objRef.current.style.minHeight = childMinHeight; + } + }, [children]); return (
diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index eea07692ba3..bcbd944da88 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -56,25 +56,27 @@ export const CoachMarkExample: Story = { Sync with CC - - - - - Hello - - Skip tour - Restart tour - - Command + B - This is the description - -
- 1 of 10 - - - - -
+
+ + + + + Hello + + Skip tour + Restart tour + + Command + B + This is the description + +
+ 1 of 10 + + + + +
+
@@ -135,25 +137,27 @@ export const CoachMarkSlider: Story = { - - - - - Hello - - Skip tour - Restart tour - - Command + B - This is the description - -
- 1 of 10 - - - - -
+
+ + + + + Hello + + Skip tour + Restart tour + + Command + B + This is the description + +
+ 1 of 10 + + + + +
+
@@ -175,25 +179,27 @@ export const CoachMarkButton: Story = { - - - - - Hello - - Skip tour - Restart tour - - Command + B - This is the description - -
- 1 of 10 - - - - -
+
+ + + + + Hello + + Skip tour + Restart tour + + Command + B + This is the description + +
+ 1 of 10 + + + + +
+
From 5b6ae363c317070cba0d92dd7be2d612bea72f40 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Mon, 14 Apr 2025 13:42:21 +1000 Subject: [PATCH 15/23] fix lint --- packages/@react-spectrum/s2/src/CoachMark.tsx | 16 +++++----------- .../s2/stories/CoachMark.stories.tsx | 2 +- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index 34984e75dc2..c010c2c8f41 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -13,22 +13,16 @@ import {ActionMenuContext} from './ActionMenu'; import { DialogTriggerProps as AriaDialogTriggerProps, - Popover as AriaPopover, ContextValue, DEFAULT_SLOT, - DialogContext, OverlayTriggerStateContext, - PopoverContext, PopoverProps, Provider, - RootMenuTriggerStateContext, useContextProps } from 'react-aria-components'; import {ButtonContext} from './Button'; -import {card, Card} from './Card'; import {CheckboxContext} from './Checkbox'; -import {colorScheme, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; -import {ColorSchemeContext} from './Provider'; +import coachmarkCss from './CoachMark.module.css'; import {ContentContext, FooterContext, KeyboardContext, TextContext} from './Content'; import { createContext, @@ -41,17 +35,15 @@ import { } from 'react'; import {DividerContext} from './Divider'; import {forwardRefType} from './types'; +import {getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import {ImageContext} from './Image'; import {ImageCoordinator} from './ImageCoordinator'; import {keyframes, raw} from '../style/style-macro' with {type: 'macro'}; -import {mergeStyles} from '../style/runtime'; -import {PressResponder} from '@react-aria/interactions'; import {SliderContext} from './Slider'; import {space, style} from '../style' with {type: 'macro'}; import {useId, useObjectRef, useOverlayPosition, useOverlayTrigger} from 'react-aria'; import {useLayoutEffect} from '@react-aria/utils'; import {useMenuTriggerState} from '@react-stately/menu'; -import coachmarkCss from './CoachMark.module.css'; export interface CoachMarkProps extends Omit, StyleProps { /** The children of the coach mark. */ @@ -245,6 +237,8 @@ export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef(null)); + triggerRef = triggerRef ?? fallbackTriggerRef; let children = ( }>({}); +const InternalCoachMarkContext = createContext<{triggerRef?: RefObject}>({}); export interface CoachMarkTriggerProps extends AriaDialogTriggerProps { } diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index bcbd944da88..22e95e286dc 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -26,11 +26,11 @@ import { Slider, Text } from '../src'; +import {card} from '../src/Card'; import Filter from '../s2wf-icons/S2_Icon_Filter_20_N.svg'; import type {Meta, StoryObj} from '@storybook/react'; import {style} from '../style' with {type: 'macro'}; import {useState} from 'react'; -import { card } from '../src/Card'; const meta: Meta = { component: CoachMark, From 3e4d2e1848e594a9866889cbb4f076c192e02b60 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Mon, 14 Apr 2025 16:28:19 +1000 Subject: [PATCH 16/23] fix tests --- .../s2/test/CoachMark.test.tsx | 72 ++++++++++++------- 1 file changed, 45 insertions(+), 27 deletions(-) diff --git a/packages/@react-spectrum/s2/test/CoachMark.test.tsx b/packages/@react-spectrum/s2/test/CoachMark.test.tsx index a413efba824..611f1f99ba5 100644 --- a/packages/@react-spectrum/s2/test/CoachMark.test.tsx +++ b/packages/@react-spectrum/s2/test/CoachMark.test.tsx @@ -25,7 +25,8 @@ import { MenuItem, Text } from '../src'; -import React from 'react'; +import {card} from '../src/Card'; +import React, {createRef, useState} from 'react'; import userEvent, {UserEvent} from '@testing-library/user-event'; const mockAnimations = () => { @@ -45,35 +46,52 @@ describe('CoachMark', () => { act(() => {jest.runAllTimers();}); }); + function CoachMarkTest(props) { + let [isOpen, setIsOpen] = useState(false); + return ( +
+ + + Sync with CC + +
+ + + + + Hello + + Skip tour + Restart tour + + Command + B + This is the description + +
+ 1 of 10 + + +
+
+
+
+
+ ); + } + it('renders a coachmark', async () => { - let onPress = jest.fn(); + let coachmarkRef = createRef(); let {getAllByRole} = render( - - Sync with CC - - - - - - Hello - - Skip tour - Restart tour - - Command + B - This is the description - -
- 1 of 10 - - -
-
-
+ ); + coachmarkRef.current!.showPopover = jest.fn(); + coachmarkRef.current!.hidePopover = jest.fn(); + let startButton = getAllByRole('button')[0]; + await user?.click(startButton); act(() => {jest.runAllTimers();}); - expect(getAllByRole('button').length).toBe(4); // 2 Dismiss + 2 actions - await user?.click(getAllByRole('button')[2]); - expect(onPress).toHaveBeenCalled(); + expect(coachmarkRef.current!.showPopover).toHaveBeenCalled(); + expect(getAllByRole('button').length).toBe(4); // start, action menu, previous, next + await user?.click(getAllByRole('button')[3]); + expect(coachmarkRef.current!.hidePopover).toHaveBeenCalled(); }); }); From 78eca49bc0fdeebd3b499f5f562a2b26ab5ba210 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 15 Apr 2025 13:18:42 +1000 Subject: [PATCH 17/23] Remove card to just examples --- packages/@react-spectrum/s2/src/CoachMark.tsx | 160 +-------------- .../s2/stories/CoachMark.stories.tsx | 184 +++++++++++++++++- .../s2/test/CoachMark.test.tsx | 36 +--- 3 files changed, 183 insertions(+), 197 deletions(-) diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index c010c2c8f41..2e05a78a576 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -10,11 +10,10 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ -import {ActionMenuContext} from './ActionMenu'; + import { DialogTriggerProps as AriaDialogTriggerProps, ContextValue, - DEFAULT_SLOT, OverlayTriggerStateContext, PopoverProps, Provider, @@ -23,7 +22,6 @@ import { import {ButtonContext} from './Button'; import {CheckboxContext} from './Checkbox'; import coachmarkCss from './CoachMark.module.css'; -import {ContentContext, FooterContext, KeyboardContext, TextContext} from './Content'; import { createContext, ForwardedRef, @@ -33,14 +31,11 @@ import { useContext, useRef } from 'react'; -import {DividerContext} from './Divider'; import {forwardRefType} from './types'; import {getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; -import {ImageContext} from './Image'; -import {ImageCoordinator} from './ImageCoordinator'; import {keyframes, raw} from '../style/style-macro' with {type: 'macro'}; import {SliderContext} from './Slider'; -import {space, style} from '../style' with {type: 'macro'}; +import {style} from '../style' with {type: 'macro'}; import {useId, useObjectRef, useOverlayPosition, useOverlayTrigger} from 'react-aria'; import {useLayoutEffect} from '@react-aria/utils'; import {useMenuTriggerState} from '@react-stately/menu'; @@ -109,125 +104,6 @@ let popover = style({ isolation: 'isolate' }, getAllowedOverrides()); -const image = style({ - width: 'full', - aspectRatio: '[3/2]', - objectFit: 'cover', - userSelect: 'none', - pointerEvents: 'none' -}); - -let title = style({ - font: 'title', - fontSize: { - size: { - XS: 'title-xs', - S: 'title-xs', - M: 'title-sm', - L: 'title', - XL: 'title-lg' - } - }, - lineClamp: 3, - gridArea: 'title' -}); - -let description = style({ - font: 'body', - fontSize: { - size: { - XS: 'body-2xs', - S: 'body-2xs', - M: 'body-xs', - L: 'body-sm', - XL: 'body' - } - }, - lineClamp: 3, - gridArea: 'description' -}); - -let keyboard = style({ - gridArea: 'keyboard', - font: 'ui', - fontWeight: 'light', - color: 'gray-600', - background: 'gray-25', - unicodeBidi: 'plaintext' -}); - -let steps = style({ - font: 'detail', - fontSize: 'detail-sm', - alignSelf: 'center' -}); - -let content = style({ - display: 'grid', - // By default, all elements are displayed in a stack. - // If an action menu is present, place it next to the title. - gridTemplateColumns: { - default: ['1fr'], - ':has([data-slot=menu])': ['minmax(0, 1fr)', 'auto'] - }, - gridTemplateAreas: { - default: [ - 'title keyboard', - 'description keyboard' - ], - ':has([data-slot=menu])': [ - 'title menu', - 'keyboard keyboard', - 'description description' - ] - }, - columnGap: 4, - flexGrow: 1, - alignItems: 'baseline', - alignContent: 'space-between', - rowGap: { - size: { - XS: 4, - S: 4, - M: space(6), - L: space(6), - XL: 8 - } - }, - paddingTop: { - default: '--card-spacing', - ':first-child': 0 - }, - paddingBottom: { - default: '[calc(var(--card-spacing) * 1.5 / 2)]', - ':last-child': 0 - } -}); - -let actionMenu = style({ - gridArea: 'menu', - // Don't cause the row to expand, preserve gap between title and description text. - // Would use -100% here but it doesn't work in Firefox. - marginY: '[calc(-1 * self(height))]' -}); - -let footer = style({ - display: 'flex', - flexDirection: 'row', - alignItems: 'end', - justifyContent: 'space-between', - gap: 8, - paddingTop: '[calc(var(--card-spacing) * 1.5 / 2)]' -}); - -const actionButtonSize = { - XS: 'XS', - S: 'XS', - M: 'S', - L: 'M', - XL: 'L' -} as const; - export const CoachMarkContext = createContext>({}); export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef) => { @@ -240,36 +116,6 @@ export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef(null)); triggerRef = triggerRef ?? fallbackTriggerRef; - let children = ( - - - {props.children} - - - ); - let {overlayProps, placement} = useOverlayPosition({ targetRef: triggerRef, overlayRef: popoverRef, @@ -302,7 +148,7 @@ export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef {/* }// Reset OverlayTriggerStateContext so the buttons inside the dialog don't retain their hover state. */} - {children} + {props.children}
); diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index 22e95e286dc..fb0b8a70c19 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -12,6 +12,7 @@ import { ActionButton, ActionMenu, + ActionMenuContext, Button, ButtonGroup, CardPreview, @@ -19,17 +20,25 @@ import { UNSTABLE_CoachMark as CoachMark, UNSTABLE_CoachMarkTrigger as CoachMarkTrigger, Content, + ContentContext, + DividerContext, Footer, + FooterContext, Image, + ImageContext, + ImageCoordinator, Keyboard, + KeyboardContext, MenuItem, Slider, - Text + Text, + TextContext } from '../src'; import {card} from '../src/Card'; +import {DEFAULT_SLOT, Provider} from 'react-aria-components'; import Filter from '../s2wf-icons/S2_Icon_Filter_20_N.svg'; import type {Meta, StoryObj} from '@storybook/react'; -import {style} from '../style' with {type: 'macro'}; +import {space, style} from '../style' with {type: 'macro'}; import {useState} from 'react'; const meta: Meta = { @@ -56,7 +65,7 @@ export const CoachMarkExample: Story = { Sync with CC -
+ @@ -76,7 +85,7 @@ export const CoachMarkExample: Story = { -
+
@@ -98,7 +107,7 @@ function ControlledCoachMark(args) { Sync with CC -
+ @@ -111,7 +120,7 @@ function ControlledCoachMark(args) { Command + B This is the description -
+
@@ -137,7 +146,7 @@ export const CoachMarkSlider: Story = { -
+ @@ -157,7 +166,7 @@ export const CoachMarkSlider: Story = { -
+
@@ -179,7 +188,7 @@ export const CoachMarkButton: Story = { -
+ @@ -199,7 +208,7 @@ export const CoachMarkButton: Story = { -
+
@@ -211,3 +220,158 @@ export const CoachMarkButton: Story = { } } }; + +function CoachMarkCard(props) { + let {size = 'M'} = props; + return ( +
+ + + {props.children} + + +
+ ); +} + + +const image = style({ + width: 'full', + aspectRatio: '[3/2]', + objectFit: 'cover', + userSelect: 'none', + pointerEvents: 'none' +}); + +let title = style({ + font: 'title', + fontSize: { + size: { + XS: 'title-xs', + S: 'title-xs', + M: 'title-sm', + L: 'title', + XL: 'title-lg' + } + }, + lineClamp: 3, + gridArea: 'title' +}); + +let description = style({ + font: 'body', + fontSize: { + size: { + XS: 'body-2xs', + S: 'body-2xs', + M: 'body-xs', + L: 'body-sm', + XL: 'body' + } + }, + lineClamp: 3, + gridArea: 'description' +}); + +let keyboard = style({ + gridArea: 'keyboard', + font: 'ui', + fontWeight: 'light', + color: 'gray-600', + background: 'gray-25', + unicodeBidi: 'plaintext' +}); + +let steps = style({ + font: 'detail', + fontSize: 'detail-sm', + alignSelf: 'center' +}); + +let content = style({ + display: 'grid', + // By default, all elements are displayed in a stack. + // If an action menu is present, place it next to the title. + gridTemplateColumns: { + default: ['1fr'], + ':has([data-slot=menu])': ['minmax(0, 1fr)', 'auto'] + }, + gridTemplateAreas: { + default: [ + 'title keyboard', + 'description keyboard' + ], + ':has([data-slot=menu])': [ + 'title menu', + 'keyboard keyboard', + 'description description' + ] + }, + columnGap: 4, + flexGrow: 1, + alignItems: 'baseline', + alignContent: 'space-between', + rowGap: { + size: { + XS: 4, + S: 4, + M: space(6), + L: space(6), + XL: 8 + } + }, + paddingTop: { + default: '--card-spacing', + ':first-child': 0 + }, + paddingBottom: { + default: '[calc(var(--card-spacing) * 1.5 / 2)]', + ':last-child': 0 + } +}); + +let actionMenu = style({ + gridArea: 'menu', + // Don't cause the row to expand, preserve gap between title and description text. + // Would use -100% here but it doesn't work in Firefox. + marginY: '[calc(-1 * self(height))]' +}); + +let footer = style({ + display: 'flex', + flexDirection: 'row', + alignItems: 'end', + justifyContent: 'space-between', + gap: 8, + paddingTop: '[calc(var(--card-spacing) * 1.5 / 2)]' +}); + +const actionButtonSize = { + XS: 'XS', + S: 'XS', + M: 'S', + L: 'M', + XL: 'L' +} as const; diff --git a/packages/@react-spectrum/s2/test/CoachMark.test.tsx b/packages/@react-spectrum/s2/test/CoachMark.test.tsx index 611f1f99ba5..2762485e7d0 100644 --- a/packages/@react-spectrum/s2/test/CoachMark.test.tsx +++ b/packages/@react-spectrum/s2/test/CoachMark.test.tsx @@ -12,20 +12,11 @@ import {act, pointerMap, render} from '@react-spectrum/test-utils-internal'; import { - ActionMenu, Button, - CardPreview, Checkbox, UNSTABLE_CoachMark as CoachMark, - UNSTABLE_CoachMarkTrigger as CoachMarkTrigger, - Content, - Footer, - Image, - Keyboard, - MenuItem, - Text + UNSTABLE_CoachMarkTrigger as CoachMarkTrigger } from '../src'; -import {card} from '../src/Card'; import React, {createRef, useState} from 'react'; import userEvent, {UserEvent} from '@testing-library/user-event'; @@ -54,24 +45,9 @@ describe('CoachMark', () => { Sync with CC -
- - - - - Hello - - Skip tour - Restart tour - - Command + B - This is the description - -
- 1 of 10 - - -
+
+ +
@@ -90,8 +66,8 @@ describe('CoachMark', () => { await user?.click(startButton); act(() => {jest.runAllTimers();}); expect(coachmarkRef.current!.showPopover).toHaveBeenCalled(); - expect(getAllByRole('button').length).toBe(4); // start, action menu, previous, next - await user?.click(getAllByRole('button')[3]); + expect(getAllByRole('button').length).toBe(3); // start, previous, next + await user?.click(getAllByRole('button')[2]); expect(coachmarkRef.current!.hidePopover).toHaveBeenCalled(); }); }); From 8b42b178d8172ad17f8c1151b636567dc182e162 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 15 Apr 2025 15:55:36 +1000 Subject: [PATCH 18/23] fix types, combine stories, add esc close --- .../s2/src/CoachMark.module.css | 5 +- packages/@react-spectrum/s2/src/CoachMark.tsx | 149 ++++++++---------- .../s2/stories/CoachMark.stories.tsx | 147 +++++------------ 3 files changed, 107 insertions(+), 194 deletions(-) diff --git a/packages/@react-spectrum/s2/src/CoachMark.module.css b/packages/@react-spectrum/s2/src/CoachMark.module.css index 98e4b20dde1..6d48bc984f4 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.module.css +++ b/packages/@react-spectrum/s2/src/CoachMark.module.css @@ -4,6 +4,8 @@ transition: display allow-discrete 200ms, overlay allow-discrete 200ms, opacity 200ms, transform 200ms; will-change: opacity, transform; opacity: 0; + animation-direction: normal; + animation-timing-function: in; &[data-placement="bottom"] { transform: translateX(0px) translateY(-4px); @@ -21,6 +23,7 @@ &:popover-open { transform: translateX(0px) translateY(0px); opacity: 1; + animation-direction: reverse; } } @@ -42,4 +45,4 @@ opacity: 0; } } -} \ No newline at end of file +} diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index 2e05a78a576..e48a5aadcef 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -36,15 +36,54 @@ import {getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro import {keyframes, raw} from '../style/style-macro' with {type: 'macro'}; import {SliderContext} from './Slider'; import {style} from '../style' with {type: 'macro'}; -import {useId, useObjectRef, useOverlayPosition, useOverlayTrigger} from 'react-aria'; +import {useId, useKeyboard, useObjectRef, useOverlayPosition, useOverlayTrigger} from 'react-aria'; import {useLayoutEffect} from '@react-aria/utils'; import {useMenuTriggerState} from '@react-stately/menu'; +import { useHasTabbableChild } from '@react-aria/focus'; -export interface CoachMarkProps extends Omit, StyleProps { - /** The children of the coach mark. */ - children: ReactNode, +const InternalCoachMarkContext = createContext<{triggerRef?: RefObject}>({}); +// TODO: decide on props +// defaultOpen, I don't think we should use it because multiple coachmarks shouldn't be open at once and it'd be too easy. +export interface CoachMarkTriggerProps extends Omit { +} + +/** + * DialogTrigger serves as a wrapper around a Dialog and its associated trigger, linking the Dialog's + * open state with the trigger's press state. Additionally, it allows you to customize the type and + * positioning of the Dialog. + */ +export function CoachMarkTrigger(props: CoachMarkTriggerProps): ReactNode { + let triggerRef = useRef(null); + // Use useMenuTriggerState instead of useOverlayTriggerState in case a menu is embedded in the dialog. + // This is needed to handle submenus. + let state = useMenuTriggerState(props); + + let {triggerProps, overlayProps} = useOverlayTrigger({type: 'dialog'}, state, triggerRef); + + // Label dialog by the trigger as a fallback if there is no title slot. + // This is done in RAC instead of hooks because otherwise we cannot distinguish + // between context and props. Normally aria-labelledby overrides the title + // but when sent by context we want the title to win. + triggerProps.id = useId(); + overlayProps['aria-labelledby'] = triggerProps.id; + delete triggerProps.onPress; - size?: 'S' | 'M' | 'L' | 'XL' + return ( + + + {props.children} + + + ); +} + +export interface CoachMarkProps extends Omit, StyleProps { + /** The children of the coach mark. */ + children: ReactNode } let popover = style({ @@ -60,46 +99,16 @@ let popover = style({ borderStyle: 'solid', borderWidth: 1, height: 'fit', + width: 'fit', + // Should we overflow? or let users decide? + overflow: 'auto', padding: 0, margin: 0, borderColor: { default: 'gray-200', forcedColors: 'ButtonBorder' }, - width: 'fit', boxSizing: 'border-box', - opacity: { - default: 0, - ':popover-open': 1 - }, - transform: { - placement: { - top: { - default: 'translateY(4px)', - ':popover-open': 'translateY(0)' - }, - bottom: { - default: 'translateY(-4px)', - ':popover-open': 'translateY(0)' - }, - left: { - default: 'translateX(4px)', - ':popover-open': 'translateX(0)' - }, - right: { - default: 'translateX(-4px)', - ':popover-open': 'translateX(0)' - } - } - }, - animationDuration: 200, - animationDirection: { - default: 'normal', - ':popover-open': 'reverse' - }, - animationTimingFunction: { - default: 'in' - }, willChange: '[opacity, transform]', isolation: 'isolate' }, getAllowedOverrides()); @@ -108,8 +117,7 @@ export const CoachMarkContext = createContext) => { [props, ref] = useContextProps(props, ref, CoachMarkContext); - let {UNSAFE_style} = props; - let {size = 'M'} = props; + let {UNSAFE_style, UNSAFE_className = ''} = props; let popoverRef = useObjectRef(ref); let state = useContext(OverlayTriggerStateContext); let {triggerRef} = useContext(InternalCoachMarkContext); @@ -134,18 +142,30 @@ export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef { + if (e.key === 'Escape') { + state?.close(); + return; + } + e.continuePropagation(); + } + }); + return (
+ className={UNSAFE_className + ' ' + coachmarkCss['coach-mark'] + popover({placement})}> {/* }// Reset OverlayTriggerStateContext so the buttons inside the dialog don't retain their hover state. */} {props.children} @@ -154,47 +174,6 @@ export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef}>({}); - -export interface CoachMarkTriggerProps extends AriaDialogTriggerProps { -} - -/** - * DialogTrigger serves as a wrapper around a Dialog and its associated trigger, linking the Dialog's - * open state with the trigger's press state. Additionally, it allows you to customize the type and - * positioning of the Dialog. - */ -export function CoachMarkTrigger(props: CoachMarkTriggerProps): ReactNode { - let triggerRef = useRef(null); - // Use useMenuTriggerState instead of useOverlayTriggerState in case a menu is embedded in the dialog. - // This is needed to handle submenus. - let state = useMenuTriggerState(props); - - let {triggerProps, overlayProps} = useOverlayTrigger({type: 'dialog'}, state, triggerRef); - - // Label dialog by the trigger as a fallback if there is no title slot. - // This is done in RAC instead of hooks because otherwise we cannot distinguish - // between context and props. Normally aria-labelledby overrides the title - // but when sent by context we want the title to win. - triggerProps.id = useId(); - overlayProps['aria-labelledby'] = triggerProps.id; - delete triggerProps.onPress; - - return ( - - - {props.children} - - - ); -} - - // TODO better way to calculate 4px transform? (not 4%?) const pulseAnimation = keyframes(` 0% { @@ -257,8 +236,10 @@ export const CoachMarkIndicator = /*#__PURE__*/ (forwardRef as forwardRefType)(f const {children, isActive} = props; let objRef = useObjectRef(ref); - // // This is very silly... better ways? can't use display: contents because it breaks positioning - // // this will break if there is a resize or different styles + // This is very silly... better ways? can't use display: contents because it breaks positioning + // this will break if there is a resize or different styles + // can't go searching for the first child because it's not always the first child, or in the case of slider, there are no border radius until the thumb and we + // definitely don't want that rounding. There may also be contextual help which would have a border radius useLayoutEffect(() => { if (objRef.current) { let styles = getComputedStyle(objRef.current.children[0]); diff --git a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx index fb0b8a70c19..61b54619e36 100644 --- a/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CoachMark.stories.tsx @@ -35,13 +35,19 @@ import { TextContext } from '../src'; import {card} from '../src/Card'; +import {CoachMarkProps, CoachMarkTriggerProps} from '../src/CoachMark'; import {DEFAULT_SLOT, Provider} from 'react-aria-components'; import Filter from '../s2wf-icons/S2_Icon_Filter_20_N.svg'; import type {Meta, StoryObj} from '@storybook/react'; +import {ReactNode, useState} from 'react'; import {space, style} from '../style' with {type: 'macro'}; -import {useState} from 'react'; -const meta: Meta = { +interface CoachMarkStoryProps extends CoachMarkProps { + coachMarkTriggerProps: CoachMarkTriggerProps, + triggerChildren: ReactNode +} + +const meta: Meta = { component: CoachMark, parameters: { layout: 'centered' @@ -50,21 +56,27 @@ const meta: Meta = { placement: { control: 'radio', options: ['top', 'left', 'left top', 'right', 'right top', 'bottom'] + }, + triggerChildren: { + table: {disable: true} } }, title: 'CoachMark' }; export default meta; -type Story = StoryObj; +type Story = StoryObj; -export const CoachMarkExample: Story = { - render: (args) => ( +function ControlledCoachMark(props: CoachMarkStoryProps) { + let {coachMarkTriggerProps, triggerChildren, ...coachMarkProps} = props; + let [isOpen, setIsOpen] = useState(false); + + return (
- - - Sync with CC - + + + {triggerChildren} + @@ -79,59 +91,26 @@ export const CoachMarkExample: Story = { This is the description
- 1 of 10 - +
- -
- ), - parameters: { - docs: { - disable: true - } - } -}; - -function ControlledCoachMark(args) { - let [isOpen, setIsOpen] = useState(false); - - return ( -
- - - Sync with CC - - - - - - - Hello - - Skip tour - Restart tour - - Command + B - This is the description - - - -
); } -export const CoachMarkRestartable: Story = { +export const CoachMarkExample: Story = { render: (args) => ( ), + args: { + triggerChildren: Sync with CC + }, parameters: { docs: { disable: true @@ -141,37 +120,11 @@ export const CoachMarkRestartable: Story = { export const CoachMarkSlider: Story = { render: (args) => ( -
- - - - - - - - - - Hello - - Skip tour - Restart tour - - Command + B - This is the description - -
- 1 of 10 - - - - -
-
-
-
- -
+ ), + args: { + triggerChildren: + }, parameters: { docs: { disable: true @@ -181,39 +134,15 @@ export const CoachMarkSlider: Story = { export const CoachMarkButton: Story = { render: (args) => ( -
- - - - - - - - - - - - Hello - - Skip tour - Restart tour - - Command + B - This is the description - -
- 1 of 10 - - - - -
-
-
-
- -
+ ), + args: { + triggerChildren: ( + + + + ) + }, parameters: { docs: { disable: true From ef0fb53baf65462726e75e1371f8e3ff3f1fddf6 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 15 Apr 2025 15:57:39 +1000 Subject: [PATCH 19/23] fix lint --- packages/@react-spectrum/s2/src/CoachMark.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index e48a5aadcef..e769920d9c3 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -39,7 +39,6 @@ import {style} from '../style' with {type: 'macro'}; import {useId, useKeyboard, useObjectRef, useOverlayPosition, useOverlayTrigger} from 'react-aria'; import {useLayoutEffect} from '@react-aria/utils'; import {useMenuTriggerState} from '@react-stately/menu'; -import { useHasTabbableChild } from '@react-aria/focus'; const InternalCoachMarkContext = createContext<{triggerRef?: RefObject}>({}); // TODO: decide on props From 59a180cf07716ce204afdcb86b8dd7c3eece19dd Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 15 Apr 2025 16:27:23 +1000 Subject: [PATCH 20/23] fix menu --- packages/@react-spectrum/s2/src/CoachMark.tsx | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index e769920d9c3..15ecf85d90c 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -36,6 +36,7 @@ import {getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro import {keyframes, raw} from '../style/style-macro' with {type: 'macro'}; import {SliderContext} from './Slider'; import {style} from '../style' with {type: 'macro'}; +import {UNSAFE_PortalProvider} from '@react-aria/overlays'; import {useId, useKeyboard, useObjectRef, useOverlayPosition, useOverlayTrigger} from 'react-aria'; import {useLayoutEffect} from '@react-aria/utils'; import {useMenuTriggerState} from '@react-stately/menu'; @@ -135,8 +136,10 @@ export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef { if (state?.isOpen && !prevOpen.current) { popoverRef.current?.showPopover(); + internalContainer.current?.showPopover(); } else if (!state?.isOpen && prevOpen.current) { popoverRef.current?.hidePopover(); + internalContainer.current?.hidePopover(); } prevOpen.current = state?.isOpen ?? false; }, [state?.isOpen]); @@ -150,6 +153,10 @@ export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef(null); return (
{/* }// Reset OverlayTriggerStateContext so the buttons inside the dialog don't retain their hover state. */} - {props.children} + internalContainer.current}> + {props.children} + +
); From c4571a5cbd1a16c8d9c8282d55dc78a611043067 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 15 Apr 2025 16:30:07 +1000 Subject: [PATCH 21/23] fix style call --- packages/@react-spectrum/s2/src/CoachMark.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index 15ecf85d90c..f4c04711c9a 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -171,7 +171,7 @@ export const CoachMark = forwardRef((props: CoachMarkProps, ref: ForwardedRef + className={UNSAFE_className + ' ' + coachmarkCss['coach-mark'] + popover}> {/* }// Reset OverlayTriggerStateContext so the buttons inside the dialog don't retain their hover state. */} internalContainer.current}> From 65c4add55e9bfdac038991408f1558208368f044 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 15 Apr 2025 16:53:12 +1000 Subject: [PATCH 22/23] fix test --- packages/@react-spectrum/s2/test/CoachMark.test.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/@react-spectrum/s2/test/CoachMark.test.tsx b/packages/@react-spectrum/s2/test/CoachMark.test.tsx index 2762485e7d0..081339cdc90 100644 --- a/packages/@react-spectrum/s2/test/CoachMark.test.tsx +++ b/packages/@react-spectrum/s2/test/CoachMark.test.tsx @@ -60,8 +60,14 @@ describe('CoachMark', () => { let {getAllByRole} = render( ); - coachmarkRef.current!.showPopover = jest.fn(); - coachmarkRef.current!.hidePopover = jest.fn(); + let popovers = document.querySelectorAll('[popover="manual"]'); + for (let popover of popovers) { + // @ts-ignore + popover.showPopover = jest.fn(); + // @ts-ignore + popover.hidePopover = jest.fn(); + } + let startButton = getAllByRole('button')[0]; await user?.click(startButton); act(() => {jest.runAllTimers();}); From 03d2fa3309f207ca34fa6ac007b903dd58b38b6b Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 15 Apr 2025 16:55:40 +1000 Subject: [PATCH 23/23] undo theme change --- packages/@react-spectrum/s2/style/spectrum-theme.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@react-spectrum/s2/style/spectrum-theme.ts b/packages/@react-spectrum/s2/style/spectrum-theme.ts index ee2125fe721..38ff6ce7b1a 100644 --- a/packages/@react-spectrum/s2/style/spectrum-theme.ts +++ b/packages/@react-spectrum/s2/style/spectrum-theme.ts @@ -350,7 +350,6 @@ const transitionProperty = { // var(--gp) is generated by the backgroundImage property when setting a gradient. // It includes a list of all of the custom properties used for each color stop. default: 'color, background-color, var(--gp), border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, translate, scale, rotate, filter, backdrop-filter', - display: 'display', colors: 'color, background-color, var(--gp), border-color, text-decoration-color, fill, stroke', opacity: 'opacity', shadow: 'box-shadow',