diff --git a/.changeset/real-lamps-teach.md b/.changeset/real-lamps-teach.md new file mode 100644 index 0000000000..2baa99d8ea --- /dev/null +++ b/.changeset/real-lamps-teach.md @@ -0,0 +1,5 @@ +--- +'@leafygreen-ui/guide-cue': patch +--- + +This PR updates the GuideCue focus to be on the primary button instead of the close button when opened for an improved UX diff --git a/packages/guide-cue/src/GuideCue/GuideCue.spec.tsx b/packages/guide-cue/src/GuideCue/GuideCue.spec.tsx index decb67085d..2c8d47cfc0 100644 --- a/packages/guide-cue/src/GuideCue/GuideCue.spec.tsx +++ b/packages/guide-cue/src/GuideCue/GuideCue.spec.tsx @@ -185,6 +185,20 @@ describe('packages/guide-cue', () => { const body = getByText(guideCueChildren); expect(body).toBeInTheDocument(); }); + + test('primary button is focusable for focus trap targeting', async () => { + const { getByRole } = renderGuideCue({ + open: true, + }); + + await waitFor(() => { + const primaryButton = getByRole('button', { name: buttonTextDefault }); + // Verify the button exists and is in the document + expect(primaryButton).toBeInTheDocument(); + // Verify button can receive focus (tabindex is not -1) + expect(primaryButton).not.toHaveAttribute('tabindex', '-1'); + }); + }); }); describe('Multi-step tooltip', () => { @@ -352,5 +366,24 @@ describe('packages/guide-cue', () => { const numOfButtons = getAllByRole('button').length; await waitFor(() => expect(numOfButtons).toEqual(2)); }); + + test('primary button is focusable for focus trap targeting', async () => { + const { getByRole } = renderGuideCue({ + open: true, + numberOfSteps: 2, + currentStep: 1, + }); + await act(async () => { + await waitForTimeout(timeout1); + }); + + await waitFor(() => { + const primaryButton = getByRole('button', { name: buttonTextDefault }); + // Verify the button exists and is in the document + expect(primaryButton).toBeInTheDocument(); + // Verify button can receive focus (tabindex is not -1) + expect(primaryButton).not.toHaveAttribute('tabindex', '-1'); + }); + }); }); }); diff --git a/packages/guide-cue/src/GuideCue/GuideCue.tsx b/packages/guide-cue/src/GuideCue/GuideCue.tsx index ad5ace16d6..b894fd8408 100644 --- a/packages/guide-cue/src/GuideCue/GuideCue.tsx +++ b/packages/guide-cue/src/GuideCue/GuideCue.tsx @@ -53,7 +53,7 @@ function GuideCue({ : 'Next'; /** - * Determines if the stand-alone tooltip should be shown. If there are multiple steps the multip-step tooltip will be shown. + * Determines if the stand-alone tooltip should be shown. If there are multiple steps the multi-step tooltip will be shown. */ const isStandalone = numberOfSteps <= 1; @@ -66,7 +66,7 @@ function GuideCue({ setPopoverOpen(true); openTimeout = setTimeout( () => - // React 18 automatically batches all updates which appears to break the opening transition. flushSync prevents this state update from automically batching. Instead updates are made synchronously. + // React 18 automatically batches all updates which appears to break the opening transition. flushSync prevents this state update from automatically batching. Instead updates are made synchronously. flushSync(() => { // tooltip opens a little after the beacon opens setTooltipOpen(true); @@ -77,7 +77,7 @@ function GuideCue({ // Adding a timeout to the popover because if we close both the tooltip and the popover at the same time the transition is not visible. Only applies to multi-step tooltip. // tooltip closes first setTooltipOpen(false); - // beacon closes a little after the tooltip cloese + // beacon closes a little after the tooltip close closeTimeout = setTimeout(() => setPopoverOpen(false), timeout2); } @@ -143,7 +143,7 @@ function GuideCue({ // this is using the reference from the `refEl` prop to position itself against {children} ) : ( - // Multistep tooltip + // Multi-step tooltip <> + cx(closeStyles, { + [closeHoverStyles]: isDarkMode, + }); + export const contentStyles = css` margin-bottom: 16px; `; @@ -55,10 +60,23 @@ export const stepStyles: Record = { `, }; -export const tooltipMultistepStyles = css` +const tooltipMultiStepStyles = css` padding: 32px 16px 16px; `; -export const tooltipStyles = css` +const tooltipStyles = css` cursor: auto; `; + +export const getTooltipStyles = ({ + isStandalone, + tooltipClassName, +}: { + isStandalone?: boolean; + tooltipClassName?: string; +}) => + cx( + { [tooltipMultiStepStyles]: !isStandalone }, + tooltipStyles, + tooltipClassName, + ); diff --git a/packages/guide-cue/src/GuideCueTooltip/GuideCueTooltip.tsx b/packages/guide-cue/src/GuideCueTooltip/GuideCueTooltip.tsx index 51881b9082..50a4f19080 100644 --- a/packages/guide-cue/src/GuideCueTooltip/GuideCueTooltip.tsx +++ b/packages/guide-cue/src/GuideCueTooltip/GuideCueTooltip.tsx @@ -1,10 +1,8 @@ -import React from 'react'; +import React, { useRef } from 'react'; import { Options } from 'focus-trap'; import FocusTrap from 'focus-trap-react'; import { Button } from '@leafygreen-ui/button'; -import { cx } from '@leafygreen-ui/emotion'; -import { useIdAllocator } from '@leafygreen-ui/hooks'; import XIcon from '@leafygreen-ui/icon/dist/X'; import { IconButton } from '@leafygreen-ui/icon-button'; import { Theme } from '@leafygreen-ui/lib'; @@ -17,20 +15,21 @@ import { bodyThemeStyles, bodyTitleStyles, buttonStyles, - closeHoverStyles, - closeStyles, contentStyles, footerStyles, + getCloseButtonStyle, + getTooltipStyles, stepStyles, - tooltipMultistepStyles, - tooltipStyles, } from './GuideCueTooltip.styles'; const ariaLabelledby = 'guide-cue-label'; const ariaDescribedby = 'guide-cue-desc'; -const focusTrapOptions: Options = { +const getFocusTrapOptions = ( + buttonRef: React.RefObject, +): Options => ({ clickOutsideDeactivates: true, + initialFocus: () => buttonRef.current || false, checkCanFocusTrap: async trapContainers => { const results = trapContainers.map(trapContainer => { return new Promise(resolve => { @@ -45,7 +44,7 @@ const focusTrapOptions: Options = { // Return a promise that resolves when all the trap containers are able to receive focus return Promise.all(results).then(() => undefined); }, -}; +}); type GuideCueTooltipProps = Partial & { theme: Theme; @@ -77,7 +76,7 @@ function GuideCueTooltip({ handleCloseClick, ...tooltipProps }: GuideCueTooltipProps) { - const focusId = useIdAllocator({ prefix: 'guide-cue' }); + const primaryButtonRef = useRef(null); return ( <> @@ -88,22 +87,21 @@ function GuideCueTooltip({ justify={tooltipJustify} align={tooltipAlign} refEl={refEl} - className={cx( - { [tooltipMultistepStyles]: !isStandalone }, - tooltipStyles, + className={getTooltipStyles({ + isStandalone, tooltipClassName, - )} + })} onClose={onEscClose} role="dialog" aria-labelledby={ariaLabelledby} renderMode={RenderMode.TopLayer} {...tooltipProps} > - +
{!isStandalone && ( )}