From 336a5b0ad06051bd52080e5d7cd610feb56604c8 Mon Sep 17 00:00:00 2001 From: Afsal K Date: Wed, 11 Dec 2024 00:55:16 +0530 Subject: [PATCH 1/8] feat(sidepanel): implement decorator prop (#6511) * refactor(useFocus): refactor repeated useEffect code * feat(sidepanel): implement decorator prop * test(sidepanel): revert class name * test(sidepanel): test coverage for decorator * test(sidepanel): test coverage for decorator * test(sidepanel): test coverage for decorator * feat(sidepanel): decorator props changes * fix(SidePanel): include decorator in remaining stories --------- Co-authored-by: Alexander Melo --- .../src/components/SidePanel/_side-panel.scss | 19 ++++-- .../SidePanel/SidePanel.stories.jsx | 25 +++++++- .../components/SidePanel/SidePanel.test.js | 58 ++++++++++++------- .../src/components/SidePanel/SidePanel.tsx | 52 ++++++++++++----- 4 files changed, 112 insertions(+), 42 deletions(-) diff --git a/packages/ibm-products-styles/src/components/SidePanel/_side-panel.scss b/packages/ibm-products-styles/src/components/SidePanel/_side-panel.scss index ea8f7818f7..6f53924328 100644 --- a/packages/ibm-products-styles/src/components/SidePanel/_side-panel.scss +++ b/packages/ibm-products-styles/src/components/SidePanel/_side-panel.scss @@ -112,7 +112,8 @@ $action-set-block-class: #{c4p-settings.$pkg-prefix}--action-set; border-right: 1px solid $border-subtle-02; } &.#{$block-class}.#{$block-class}--has-slug, - &.#{$block-class}.#{$block-class}--has-ai-label { + &.#{$block-class}.#{$block-class}--has-ai-label, + &.#{$block-class}.#{$block-class}--has-decorator { border-color: transparent; box-shadow: inset 0 -80px 70px -65px $ai-inner-shadow, 0 4px 10px 2px $ai-drop-shadow; @@ -199,14 +200,16 @@ $action-set-block-class: #{c4p-settings.$pkg-prefix}--action-set; &.#{$block-class}:has(.#{$block-class}__action-toolbar), &.#{$block-class}--has-action-toolbar, &.#{$block-class}--has-slug, - &.#{$block-class}--has-ai-label { + &.#{$block-class}--has-ai-label, + &.#{$block-class}--has-decorator { --#{$block-class}--title-padding-right: #{$spacing-10}; } &.#{$block-class}:has(.#{$block-class}__action-toolbar), &.#{$block-class}--has-action-toolbar { &.#{$block-class}--has-slug, - &.#{$block-class}--has-ai-label { + &.#{$block-class}--has-ai-label, + &.#{$block-class}--has-decorator { --#{$block-class}--title-padding-right: #{$spacing-11}; } } @@ -313,7 +316,8 @@ $action-set-block-class: #{c4p-settings.$pkg-prefix}--action-set; } &.#{$block-class}--has-slug .#{$block-class}--scrolls, - &.#{$block-class}--has-ai-label .#{$block-class}--scrolls { + &.#{$block-class}--has-ai-label .#{$block-class}--scrolls, + &.#{$block-class}--has-decorator .#{$block-class}--scrolls { @include utilities.ai-popover-gradient('default', 0, 'layer'); box-shadow: inset 0 -80px 70px -65px $ai-inner-shadow, @@ -368,7 +372,8 @@ $action-set-block-class: #{c4p-settings.$pkg-prefix}--action-set; } .#{$block-class}__slug-and-close, - .#{$block-class}__ai-label-and-close { + .#{$block-class}__ai-label-and-close, + .#{$block-class}__decorator-and-close { position: absolute; z-index: 10; /* must be higher than title container border bottom */ top: 0; @@ -470,7 +475,9 @@ $action-set-block-class: #{c4p-settings.$pkg-prefix}--action-set; inset: 0; } +/* stylelint-disable-next-line carbon/theme-token-use */ .#{$block-class}--has-slug + .#{$block-class}__overlay, -.#{$block-class}--has-ai-label + .#{$block-class}__overlay { +.#{$block-class}--has-ai-label + .#{$block-class}__overlay, +.#{$block-class}--has-decorator + .#{$block-class}__overlay { background-color: $ai-overlay; } diff --git a/packages/ibm-products/src/components/SidePanel/SidePanel.stories.jsx b/packages/ibm-products/src/components/SidePanel/SidePanel.stories.jsx index b177af89f6..62417f052b 100644 --- a/packages/ibm-products/src/components/SidePanel/SidePanel.stories.jsx +++ b/packages/ibm-products/src/components/SidePanel/SidePanel.stories.jsx @@ -446,6 +446,19 @@ export default { 'Optional prop that is intended for any scenario where something is being generated by AI to reinforce AI transparency, accountability, and explainability at the UI level.', options: [0, 1], }, + decorator: { + control: { + type: 'select', + labels: { + 0: 'No AI Label', + 1: 'with AI Label', + }, + default: 0, + }, + description: + 'Optional prop that is intended for any scenario where something is being generated by AI to reinforce AI transparency, accountability, and explainability at the UI level.', + options: [0, 1], + }, }, decorators: [sidePanelDecorator(renderUIShellHeader, prefix)], }; @@ -456,6 +469,7 @@ const SlideOverTemplate = ({ actions, aiLabel, slug, + decorator, ...args }) => { const [open, setOpen] = useState(false); @@ -479,6 +493,7 @@ const SlideOverTemplate = ({ ref={testRef} aiLabel={aiLabel && sampleAILabel} slug={slug && sampleAILabel} + decorator={decorator && sampleAILabel} launcherButtonRef={buttonRef} > {!minimalContent && } @@ -492,6 +507,7 @@ const FirstElementDisabledTemplate = ({ actions, aiLabel, slug, + decorator, ...args }) => { const [open, setOpen] = useState(false); @@ -515,6 +531,7 @@ const FirstElementDisabledTemplate = ({ ref={testRef} aiLabel={aiLabel && sampleAILabel} slug={slug && sampleAILabel} + decorator={decorator && sampleAILabel} launcherButtonRef={buttonRef} > {!minimalContent && ( @@ -553,7 +570,7 @@ const FirstElementDisabledTemplate = ({ }; // eslint-disable-next-line react/prop-types -const StepTemplate = ({ actions, aiLabel, slug, ...args }) => { +const StepTemplate = ({ actions, aiLabel, slug, decorator, ...args }) => { const [open, setOpen] = useState(false); const [currentStep, setCurrentStep] = useState(0); const buttonRef = useRef(undefined); @@ -576,6 +593,7 @@ const StepTemplate = ({ actions, aiLabel, slug, ...args }) => { actions={actionSets[actions]} aiLabel={aiLabel && sampleAILabel} slug={slug && sampleAILabel} + decorator={decorator && sampleAILabel} launcherButtonRef={buttonRef} > { }; // eslint-disable-next-line react/prop-types -const SlideInTemplate = ({ actions, aiLabel, slug, ...args }) => { +const SlideInTemplate = ({ actions, aiLabel, slug, decorator, ...args }) => { const [open, setOpen] = useState(false); const buttonRef = useRef(undefined); @@ -612,6 +630,7 @@ const SlideInTemplate = ({ actions, aiLabel, slug, ...args }) => { actions={actionSets[actions]} aiLabel={aiLabel && sampleAILabel} slug={slug && sampleAILabel} + decorator={decorator && sampleAILabel} launcherButtonRef={buttonRef} > @@ -675,7 +694,7 @@ export const WithAILabel = SlideOverTemplate.bind({}); WithAILabel.args = { includeOverlay: true, actions: 0, - aiLabel: 1, + decorator: 1, ...defaultStoryProps, }; diff --git a/packages/ibm-products/src/components/SidePanel/SidePanel.test.js b/packages/ibm-products/src/components/SidePanel/SidePanel.test.js index 2df6584256..5e98d445ca 100644 --- a/packages/ibm-products/src/components/SidePanel/SidePanel.test.js +++ b/packages/ibm-products/src/components/SidePanel/SidePanel.test.js @@ -38,6 +38,26 @@ const selectorPageContentValue = '#side-panel-test-page-content'; const onRequestCloseFn = jest.fn(); const onUnmountFn = jest.fn(); +const sampleAILabel = ( + + +
+

AI Explained

+

84%

+

Confidence score

+

+ This is not really Lorem Ipsum but the spell checker did not like the + previous text with it's non-words which is why this unwieldy + sentence, should one choose to call it that, here. +

+
+

Model type

+

Foundation model

+
+ + +); + const renderSidePanel = ({ ...rest } = {}, children =

test

) => render( { ); expect(navigationAction).toBeTruthy(); }); - it('should not have AI Label when it is not passed', () => { + + it('should have AI Label when it is passed through slug', () => { + const { container } = renderSidePanel({ + slug: sampleAILabel, + }); + expect(container.querySelector('.aiLabel-container')).toBeTruthy(); + }); + + it('should not have a ai label container when a it is not passed', () => { const { container } = renderSidePanel(); expect(container.querySelector('.aiLabel-container')).toBe(null); }); + it('should have AI Label when it is passed', () => { - const sampleAILabel = ( - - -
-

AI Explained

-

84%

-

Confidence score

-

- This is not really Lorem Ipsum but the spell checker did not like - the previous text with it's non-words which is why this - unwieldy sentence, should one choose to call it that, here. -

-
-

Model type

-

Foundation model

-
- - - ); const { container } = renderSidePanel({ aiLabel: sampleAILabel, }); expect(container.querySelector('.aiLabel-container')).toBeTruthy(); }); + + it('should have AI Label when it is passed to decorator', () => { + const { container } = renderSidePanel({ + decorator: sampleAILabel, + }); + expect(container.querySelector('.aiLabel-container')).toBeTruthy(); + }); + it('should throw console warning if labelText passed without Title', () => { const consoleWarnSpy = jest .spyOn(console, 'warn') diff --git a/packages/ibm-products/src/components/SidePanel/SidePanel.tsx b/packages/ibm-products/src/components/SidePanel/SidePanel.tsx index 76534937c3..94c020c21c 100644 --- a/packages/ibm-products/src/components/SidePanel/SidePanel.tsx +++ b/packages/ibm-products/src/components/SidePanel/SidePanel.tsx @@ -170,16 +170,22 @@ type SidePanelBaseProps = { slideIn?: boolean; /** - * @deprecated please use the `aiLabel` prop + * @deprecated please use the `decorator` instead * **Experimental:** Provide a `Slug` component to be rendered inside the `SidePanel` component */ slug?: ReactNode; /** + * @deprecated please use the `decorator` instead * Optional prop that is intended for any scenario where something is being generated by AI to reinforce AI transparency, accountability, and explainability at the UI level. */ aiLabel?: ReactNode; + /** + * Provide a `decorator` component to be rendered inside the `SidePanel` component + */ + decorator?: ReactNode; + /** * Sets the subtitle text */ @@ -247,6 +253,7 @@ export let SidePanel = React.forwardRef( closeIconDescription = defaults.closeIconDescription, condensedActions, currentStep = defaults.currentStep, + decorator, id = blockClass, includeOverlay, labelText, @@ -670,7 +677,9 @@ export let SidePanel = React.forwardRef( [`${blockClass}--right-placement`]: placement === 'right', [`${blockClass}--left-placement`]: placement === 'left', [`${blockClass}--slide-in`]: slideIn, - [`${blockClass}--has-ai-label`]: !!aiLabel || !!slug, + [`${blockClass}--has-decorator`]: decorator, + [`${blockClass}--has-slug`]: slug, + [`${blockClass}--has-ai-label`]: aiLabel, [`${blockClass}--condensed-actions`]: condensedActions, [`${blockClass}--has-overlay`]: includeOverlay, }, @@ -704,29 +713,39 @@ export let SidePanel = React.forwardRef( ); const renderHeader = () => { - const aiLabelCloseSize = + const closeSize = actions && actions.length && /l/.test(size) ? 'md' : 'sm'; - let normalizedAILabel; + let normalizedDecorator; /** * slug is deprecated * can remove this condition in future release */ - if (slug && slug['type']?.displayName === 'Slug') { - normalizedAILabel = React.cloneElement( + if (slug && slug['type']?.displayName === 'AILabel') { + normalizedDecorator = React.cloneElement( slug as React.ReactElement, { // slug size is sm unless actions and size > md - size: aiLabelCloseSize, + size: closeSize, } ); } if (aiLabel && aiLabel['type']?.displayName === 'AILabel') { - normalizedAILabel = React.cloneElement( + normalizedDecorator = React.cloneElement( aiLabel as React.ReactElement, { // aiLabel size is sm unless actions and size > md - size: aiLabelCloseSize, + size: closeSize, + } + ); + } + + if (decorator?.['type']?.displayName === 'AILabel') { + normalizedDecorator = React.cloneElement( + decorator as React.ReactElement, + { + // decorator size is sm unless actions and size > md + size: closeSize, } ); } @@ -745,7 +764,7 @@ export let SidePanel = React.forwardRef( {currentStep > 0 && ( + +
+ {mainText} + +
+ + + ); +}; + // These are tests than apply to both Tearsheet and TearsheetNarrow // and also (with extra props and omitting button tests) to CreateTearsheetNarrow let tooManyButtonsTestedAlready = false; @@ -262,40 +298,6 @@ const commonTests = (Ts, name, props, testActions) => { }); it('should return focus to the launcher button', async () => { - const mainText = 'Main content 1'; - const inputId = 'stacked-input-1'; - - // eslint-disable-next-line react/prop-types - const DummyComponent = ({ open }) => { - const buttonRef = React.useRef(undefined); - - return ( - <> - - -
- {mainText} - -
- - - ); - }; - const { rerender, getByText, getByTestId } = render( ); @@ -320,6 +322,19 @@ const commonTests = (Ts, name, props, testActions) => { await act(() => new Promise((resolve) => setTimeout(resolve, 0))); expect(launchButtonEl).toHaveFocus(); }); + + it('should call onBlur only once', async () => { + const { getByTestId } = render(); + + const inputEl = getByTestId(inputId); + const closeButton = screen.getByRole('button', { + name: closeIconDescription, + }); + + expect(inputEl).toHaveFocus(); + await act(() => userEvent.click(closeButton)); + expect(onBlur).toHaveBeenCalledTimes(1); + }); } it('is visible when open is true', async () => { diff --git a/packages/ibm-products/src/components/Tearsheet/TearsheetShell.tsx b/packages/ibm-products/src/components/Tearsheet/TearsheetShell.tsx index 74dd3e7376..707d62a460 100644 --- a/packages/ibm-products/src/components/Tearsheet/TearsheetShell.tsx +++ b/packages/ibm-products/src/components/Tearsheet/TearsheetShell.tsx @@ -65,6 +65,11 @@ interface TearsheetShellProps extends PropsWithChildren { */ className?: string; + /** + * Used to track the current step on components which use `StepsContext` and `TearsheetShell` + */ + currentStep?: number; + /** * A description of the flow, displayed in the header area of the tearsheet. */ @@ -199,13 +204,9 @@ export type CloseIconDescriptionTypes = type stackTypes = { open: Array<{ (a: number, b: number): void; - checkFocus?: () => void; - claimFocus?: () => void; }>; all: Array<{ (a: number, b: number): void; - checkFocus?: () => void; - claimFocus?: () => void; }>; sizes: Array; }; @@ -242,6 +243,7 @@ export const TearsheetShell = React.forwardRef( children, className, closeIconDescription, + currentStep, description, hasCloseIcon, headerActions, @@ -274,7 +276,7 @@ export const TearsheetShell = React.forwardRef( const modalRef = (ref || localRef) as MutableRefObject; const { width } = useResizeObserver(resizer); const prevOpen = usePreviousValue(open); - const { firstElement, keyDownListener, specifiedElement } = useFocus( + const { firstElement, keyDownListener } = useFocus( modalRef, selectorPrimaryFocus ); @@ -309,29 +311,22 @@ export const TearsheetShell = React.forwardRef( setDepth(newDepth); setPosition(newPosition); } - handleStackChange.checkFocus = function () { - // if we are now the topmost tearsheet, ensure we have focus - if ( - open && - position === depth && - modalRefValue && - !modalRefValue.contains(document.activeElement) - ) { - handleStackChange.claimFocus(); - } - }; - - // Callback to give the tearsheet the opportunity to claim focus - handleStackChange.claimFocus = function () { - claimFocus(firstElement, modalRef, selectorPrimaryFocus); - }; useEffect(() => { - if (open) { + if (open && position === depth) { // Focusing the first element or selectorPrimaryFocus element claimFocus(firstElement, modalRef, selectorPrimaryFocus); } - }, [firstElement, modalRef, open, selectorPrimaryFocus]); + }, [ + currentStep, + depth, + firstElement, + modalRef, + modalRefValue, + open, + position, + selectorPrimaryFocus, + ]); useEffect(() => { if (prevOpen && !open && launcherButtonRef) { @@ -341,24 +336,6 @@ export const TearsheetShell = React.forwardRef( } }, [launcherButtonRef, open, prevOpen]); - useEffect(() => { - if (open && position !== depth) { - setTimeout(() => { - if (selectorPrimaryFocus) { - return specifiedElement?.focus(); - } - firstElement?.focus(); - }, 0); - } - }, [ - position, - depth, - firstElement, - open, - specifiedElement, - selectorPrimaryFocus, - ]); - useEffect(() => { const notify = () => stack.all.forEach((handler) => { @@ -366,7 +343,6 @@ export const TearsheetShell = React.forwardRef( Math.min(stack.open.length, maxDepth), stack.open.indexOf(handler) + 1 ); - handler.checkFocus?.(); }); // Register this tearsheet's stack change callback/listener. @@ -401,14 +377,6 @@ export const TearsheetShell = React.forwardRef( }; }, [open, size]); - function handleFocus() { - // If something within us is receiving focus but we are not the topmost - // stacked tearsheet, transfer focus to the topmost tearsheet instead - if (position < depth) { - stack.open[stack.open.length - 1].claimFocus?.(); - } - } - if (position <= depth) { // Include a modal header if and only if one or more of these is given. // We can't use a Wrap for the ModalHeader because ComposedModal requires @@ -464,7 +432,6 @@ export const TearsheetShell = React.forwardRef( !areAllSameSizeVariant(), })} {...{ onClose, open, selectorPrimaryFocus }} - onFocus={handleFocus} onKeyDown={keyDownListener} preventCloseOnClickOutside={!isPassive} ref={modalRef} diff --git a/packages/ibm-products/src/global/js/hooks/useFocus.js b/packages/ibm-products/src/global/js/hooks/useFocus.js index e6a7dd052a..b4cc99a601 100644 --- a/packages/ibm-products/src/global/js/hooks/useFocus.js +++ b/packages/ibm-products/src/global/js/hooks/useFocus.js @@ -122,9 +122,9 @@ export const claimFocus = ( specifiedEl && window?.getComputedStyle(specifiedEl)?.display !== 'none' ) { - return specifiedEl.focus(); + setTimeout(() => specifiedEl.focus(), 0); } + } else { + setTimeout(() => firstElement?.focus(), 0); } - - setTimeout(() => firstElement?.focus(), 0); };