From db0fd2246155ca8d982102c1e190607b7ba60523 Mon Sep 17 00:00:00 2001 From: Gregoire Cattan Date: Wed, 27 Nov 2024 10:38:11 +0100 Subject: [PATCH 01/10] feat: add property isOpenByDefault to coachmark --- .../src/components/Coachmark/Coachmark.tsx | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx index d4c012e538..fa692ecdf9 100644 --- a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx +++ b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx @@ -113,6 +113,11 @@ export interface CoachmarkProps { * Determines the theme of the component. */ theme?: 'light' | 'dark'; + /** + * If set, determines if the coachmark will be opened by default. + * Setting this value override the behavior set by the overlay kind. + */ + isOpenByDefault?: boolean; } /** @@ -136,7 +141,7 @@ export let Coachmark = forwardRef( portalTarget, target, theme = defaults.theme, - + isOpenByDefault, // Collect any other property values passed in. ...rest }, @@ -144,7 +149,7 @@ export let Coachmark = forwardRef( ) => { const isBeacon = overlayKind === COACHMARK_OVERLAY_KIND.TOOLTIP; const isStacked = overlayKind === COACHMARK_OVERLAY_KIND.STACKED; - const [isOpen, setIsOpen] = useState(isStacked); + const [isOpen, setIsOpen] = useState(isOpenByDefault || isStacked); const [shouldResetPosition, setShouldResetPosition] = useState(false); const [targetRect, setTargetRect] = useState(); const [targetOffset, setTargetOffset] = useState({ x: 0, y: 0 }); @@ -396,6 +401,13 @@ Coachmark.propTypes = { */ className: PropTypes.string, + /** + * If set, determines if the coachmark will be opened by default. + * This property is optional, but if set it overrides + * the behavior set by the overlay kind. + */ + isOpenByDefault: PropTypes.bool, + /** * Function to call when the Coachmark closes. */ @@ -407,6 +419,8 @@ Coachmark.propTypes = { /** * What kind or style of Coachmark to render. + * Note that stacked coachmark are opened by default, unless the property + * `isOpenByDefault` is defined. */ overlayKind: PropTypes.oneOf(['tooltip', 'floating', 'stacked']), From 3dea15dd7c856f41affa35708bfaf4e6fbb3a94d Mon Sep 17 00:00:00 2001 From: Gregoire Cattan Date: Wed, 27 Nov 2024 21:31:13 +0100 Subject: [PATCH 02/10] feat: add tests --- .../components/Coachmark/Coachmark.test.js | 24 +++++++++++++++++++ .../src/components/Coachmark/Coachmark.tsx | 18 +++++--------- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/packages/ibm-products/src/components/Coachmark/Coachmark.test.js b/packages/ibm-products/src/components/Coachmark/Coachmark.test.js index 8c4f955a08..909e9ac23c 100644 --- a/packages/ibm-products/src/components/Coachmark/Coachmark.test.js +++ b/packages/ibm-products/src/components/Coachmark/Coachmark.test.js @@ -105,4 +105,28 @@ describe(componentName, () => { componentName ); }); + + it('Check coachmark can be open by default', () => { + renderCoachmark({ + 'data-testid': dataTestId, + isOpenByDefault: true, + }); + const coachmarkContainer = screen.getByTestId(dataTestId); + const coachmarkButton = + coachmarkContainer.children[0].children[0].children[0]; + const ariaExpanded = coachmarkButton.getAttribute('aria-expanded'); + expect(ariaExpanded).toEqual('true'); + }); + + it('Check stacked coachmark are always open by default', () => { + renderCoachmark({ + 'data-testid': dataTestId, + isOpenByDefault: false, + overlayKind: 'stacked', + }); + const coachmarkContainer = screen.getByTestId(dataTestId); + const coachmarkButton = coachmarkContainer.children[0].children[0]; + const ariaExpanded = coachmarkButton.getAttribute('aria-expanded'); + expect(ariaExpanded).toEqual('true'); + }); }); diff --git a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx index fa692ecdf9..1e1d14ddac 100644 --- a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx +++ b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx @@ -38,6 +38,7 @@ const defaults = { onClose: () => {}, overlayKind: 'tooltip', theme: 'light', + isOpenByDefault: false, }; export interface CoachmarkProps { @@ -114,10 +115,10 @@ export interface CoachmarkProps { */ theme?: 'light' | 'dark'; /** - * If set, determines if the coachmark will be opened by default. - * Setting this value override the behavior set by the overlay kind. + * Determines if the coachmark is open by default. + * Does nothing is `overlayKind=slacked`. */ - isOpenByDefault?: boolean; + isOpenByDefault: boolean; } /** @@ -141,7 +142,7 @@ export let Coachmark = forwardRef( portalTarget, target, theme = defaults.theme, - isOpenByDefault, + isOpenByDefault = defaults.isOpenByDefault, // Collect any other property values passed in. ...rest }, @@ -149,7 +150,7 @@ export let Coachmark = forwardRef( ) => { const isBeacon = overlayKind === COACHMARK_OVERLAY_KIND.TOOLTIP; const isStacked = overlayKind === COACHMARK_OVERLAY_KIND.STACKED; - const [isOpen, setIsOpen] = useState(isOpenByDefault || isStacked); + const [isOpen, setIsOpen] = useState(isStacked || isOpenByDefault); const [shouldResetPosition, setShouldResetPosition] = useState(false); const [targetRect, setTargetRect] = useState(); const [targetOffset, setTargetOffset] = useState({ x: 0, y: 0 }); @@ -401,13 +402,6 @@ Coachmark.propTypes = { */ className: PropTypes.string, - /** - * If set, determines if the coachmark will be opened by default. - * This property is optional, but if set it overrides - * the behavior set by the overlay kind. - */ - isOpenByDefault: PropTypes.bool, - /** * Function to call when the Coachmark closes. */ From 85586fe4637904efddb782ea047e9b83e9e82e2d Mon Sep 17 00:00:00 2001 From: Gregoire Cattan Date: Thu, 28 Nov 2024 19:07:43 +0100 Subject: [PATCH 03/10] feat: add more comment and API documentation --- .../src/components/Coachmark/Coachmark.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx index 1e1d14ddac..86f68456a2 100644 --- a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx +++ b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx @@ -116,7 +116,7 @@ export interface CoachmarkProps { theme?: 'light' | 'dark'; /** * Determines if the coachmark is open by default. - * Does nothing is `overlayKind=slacked`. + * Does nothing if `overlayKind=slacked`. */ isOpenByDefault: boolean; } @@ -402,6 +402,12 @@ Coachmark.propTypes = { */ className: PropTypes.string, + /** + * Determines if the coachmark is open by default. + * Does nothing if `overlayKind=slacked`. + */ + isOpenByDefault: PropTypes.bool.isRequired, + /** * Function to call when the Coachmark closes. */ @@ -413,8 +419,7 @@ Coachmark.propTypes = { /** * What kind or style of Coachmark to render. - * Note that stacked coachmark are opened by default, unless the property - * `isOpenByDefault` is defined. + * Note that, `stacked` coachmarks are always open, even if `isOpenByDefault=true`. */ overlayKind: PropTypes.oneOf(['tooltip', 'floating', 'stacked']), From e0caed56d79b8fa22d4a67f394a373bdec2eb5d5 Mon Sep 17 00:00:00 2001 From: Gregoire Cattan Date: Thu, 28 Nov 2024 19:10:55 +0100 Subject: [PATCH 04/10] feat: correct typo in doc --- packages/ibm-products/src/components/Coachmark/Coachmark.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx index 86f68456a2..db41f1d099 100644 --- a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx +++ b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx @@ -419,7 +419,7 @@ Coachmark.propTypes = { /** * What kind or style of Coachmark to render. - * Note that, `stacked` coachmarks are always open, even if `isOpenByDefault=true`. + * Note that, `stacked` coachmarks are always open, even if `isOpenByDefault=false`. */ overlayKind: PropTypes.oneOf(['tooltip', 'floating', 'stacked']), From 41ce56895763f7c861cc8fe6bddd5dac63c2f2ba Mon Sep 17 00:00:00 2001 From: gcattan Date: Fri, 29 Nov 2024 10:58:17 +0100 Subject: [PATCH 05/10] Update packages/ibm-products/src/components/Coachmark/Coachmark.tsx Co-authored-by: Nandan Devadula <47176249+devadula-nandan@users.noreply.github.com> --- packages/ibm-products/src/components/Coachmark/Coachmark.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx index db41f1d099..cd05fb9ea5 100644 --- a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx +++ b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx @@ -404,7 +404,7 @@ Coachmark.propTypes = { /** * Determines if the coachmark is open by default. - * Does nothing if `overlayKind=slacked`. + * Does nothing if `overlayKind=stacked`. */ isOpenByDefault: PropTypes.bool.isRequired, From 1239e99fc597afb5e558e739040b4f741aa96d2a Mon Sep 17 00:00:00 2001 From: Gregoire Cattan Date: Mon, 2 Dec 2024 19:39:43 +0100 Subject: [PATCH 06/10] feat: refactor test --- .../src/components/Coachmark/Coachmark.test.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/ibm-products/src/components/Coachmark/Coachmark.test.js b/packages/ibm-products/src/components/Coachmark/Coachmark.test.js index 0e08e5fa73..278f8ad917 100644 --- a/packages/ibm-products/src/components/Coachmark/Coachmark.test.js +++ b/packages/ibm-products/src/components/Coachmark/Coachmark.test.js @@ -61,6 +61,13 @@ const renderCoachmark = ({ ...rest } = {}, children = childrenContent) => ); +const isCoachmarkVisible = () => { + const coachmarkContainer = screen.getByTestId(dataTestId); + const coachmarkButton = coachmarkContainer.getElementsByTagName('button')[0]; + const ariaExpanded = coachmarkButton.getAttribute('aria-expanded'); + return ariaExpanded === 'true'; +}; + describe(componentName, () => { it('renders a component Coachmark', () => { renderCoachmark({ 'data-testid': dataTestId }); @@ -209,11 +216,7 @@ describe(componentName, () => { 'data-testid': dataTestId, isOpenByDefault: true, }); - const coachmarkContainer = screen.getByTestId(dataTestId); - const coachmarkButton = - coachmarkContainer.children[0].children[0].children[0]; - const ariaExpanded = coachmarkButton.getAttribute('aria-expanded'); - expect(ariaExpanded).toEqual('true'); + expect(isCoachmarkVisible()).toBeTruthy(); }); it('Check stacked coachmark are always open by default', () => { @@ -222,9 +225,6 @@ describe(componentName, () => { isOpenByDefault: false, overlayKind: 'stacked', }); - const coachmarkContainer = screen.getByTestId(dataTestId); - const coachmarkButton = coachmarkContainer.children[0].children[0]; - const ariaExpanded = coachmarkButton.getAttribute('aria-expanded'); - expect(ariaExpanded).toEqual('true'); + expect(isCoachmarkVisible()).toBeTruthy(); }); }); From 645f031cfc7b7ae1860c506d3c6fc70fb9ef1486 Mon Sep 17 00:00:00 2001 From: Gregoire Cattan Date: Mon, 2 Dec 2024 19:43:59 +0100 Subject: [PATCH 07/10] fix: isOpenByDefault should be optional --- packages/ibm-products/src/components/Coachmark/Coachmark.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx index cd05fb9ea5..83c1cdabf3 100644 --- a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx +++ b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx @@ -118,7 +118,7 @@ export interface CoachmarkProps { * Determines if the coachmark is open by default. * Does nothing if `overlayKind=slacked`. */ - isOpenByDefault: boolean; + isOpenByDefault?: boolean; } /** @@ -406,7 +406,7 @@ Coachmark.propTypes = { * Determines if the coachmark is open by default. * Does nothing if `overlayKind=stacked`. */ - isOpenByDefault: PropTypes.bool.isRequired, + isOpenByDefault: PropTypes.bool, /** * Function to call when the Coachmark closes. From 59740885c3a9a74fda1b5b4277f49092ef6afd54 Mon Sep 17 00:00:00 2001 From: gcattan Date: Tue, 3 Dec 2024 16:00:48 +0100 Subject: [PATCH 08/10] Update packages/ibm-products/src/components/Coachmark/Coachmark.tsx Co-authored-by: Alexander Melo --- packages/ibm-products/src/components/Coachmark/Coachmark.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx index 83c1cdabf3..017b98a501 100644 --- a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx +++ b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx @@ -116,7 +116,7 @@ export interface CoachmarkProps { theme?: 'light' | 'dark'; /** * Determines if the coachmark is open by default. - * Does nothing if `overlayKind=slacked`. + * Does nothing if `overlayKind=stacked`. */ isOpenByDefault?: boolean; } From d89119e519a14134a75a94fa72ddfb9d2557a833 Mon Sep 17 00:00:00 2001 From: Gregoire Cattan Date: Wed, 4 Dec 2024 10:05:20 +0100 Subject: [PATCH 09/10] test: remove unnecessary test --- .../src/components/Coachmark/Coachmark.test.js | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/ibm-products/src/components/Coachmark/Coachmark.test.js b/packages/ibm-products/src/components/Coachmark/Coachmark.test.js index 278f8ad917..5ec8084cb6 100644 --- a/packages/ibm-products/src/components/Coachmark/Coachmark.test.js +++ b/packages/ibm-products/src/components/Coachmark/Coachmark.test.js @@ -218,13 +218,4 @@ describe(componentName, () => { }); expect(isCoachmarkVisible()).toBeTruthy(); }); - - it('Check stacked coachmark are always open by default', () => { - renderCoachmark({ - 'data-testid': dataTestId, - isOpenByDefault: false, - overlayKind: 'stacked', - }); - expect(isCoachmarkVisible()).toBeTruthy(); - }); }); From 7b4ec2bc9f12f5da0c3017a605eae2b78d57f1a2 Mon Sep 17 00:00:00 2001 From: Gregoire Cattan Date: Wed, 4 Dec 2024 13:59:18 +0100 Subject: [PATCH 10/10] fix: remove leftover --- packages/ibm-products/src/components/Coachmark/Coachmark.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx index 017b98a501..56796d415d 100644 --- a/packages/ibm-products/src/components/Coachmark/Coachmark.tsx +++ b/packages/ibm-products/src/components/Coachmark/Coachmark.tsx @@ -419,7 +419,6 @@ Coachmark.propTypes = { /** * What kind or style of Coachmark to render. - * Note that, `stacked` coachmarks are always open, even if `isOpenByDefault=false`. */ overlayKind: PropTypes.oneOf(['tooltip', 'floating', 'stacked']),