-
Notifications
You must be signed in to change notification settings - Fork 138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(coachmark): Add default opening for not-stacked coachmarks #6516
feat(coachmark): Add default opening for not-stacked coachmarks #6516
Conversation
DCO Assistant Lite bot All contributors have signed the DCO. |
✅ Deploy Preview for ibm-products-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-for-ibm-products ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I have read the DCO document and I hereby sign the DCO. |
I have read the DCO document and I hereby sign the DCO. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6516 +/- ##
==========================================
- Coverage 79.92% 79.91% -0.01%
==========================================
Files 394 394
Lines 12887 12888 +1
Branches 4265 4267 +2
==========================================
Hits 10300 10300
- Misses 2587 2588 +1
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation looks good. Could you please add a control in the component API section of the documentation to reference this prop?
Co-authored-by: Nandan Devadula <[email protected]>
@devadula-nandan thanks for the suggestion! 4 workflows are still pending, everything else looks good. |
packages/ibm-products/src/components/Coachmark/Coachmark.test.js
Outdated
Show resolved
Hide resolved
packages/ibm-products/src/components/Coachmark/Coachmark.test.js
Outdated
Show resolved
Hide resolved
packages/ibm-products/src/components/Coachmark/Coachmark.test.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Alexander Melo <[email protected]>
…ps://github.com/gcattan/ibm-products into feat/5709_default_opening_not_stacked_coachmark
As agreed in slack this morning with @devadula-nandan the last test probably doesn't make a valid point. Therefore removing it. In top of it, we discussed the duplicated logic between this "overlayKind=stacked" property and the "CoachmarkStacked". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@matthewgallo @AlexanderMelox can we merge this PR? |
@gcattan I do not have access to Percy checks neither, we prob have to wait for @matthewgallo or someone can give me access to Percy also |
I can take a quick look at Percy |
Thank you! I see that all checks are now passing. |
Thanks for contributing @gcattan! |
73ee693
Thank you all for the review! |
Closes #5709
Add a property to open the coachmark by default.
What did you change?
Add the property
isOpenByDefault
to the coachmark, which applies in the case the coachmark is not a stacked coachmark.How did you test and verify your work?
Added two tests to the suite:
isOpenByDefault
istrue
isOpenByDefault
isfalse
.