feat(tearsheet): implement enable-presence feature#8822
feat(tearsheet): implement enable-presence feature#8822makafsal wants to merge 61 commits intocarbon-design-system:mainfrom
enable-presence feature#8822Conversation
✅ Deploy Preview for ibm-products-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for carbon-for-ibm-products ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8822 +/- ##
==========================================
- Coverage 80.57% 80.17% -0.41%
==========================================
Files 426 428 +2
Lines 19541 19593 +52
Branches 4278 4274 -4
==========================================
- Hits 15746 15709 -37
- Misses 3795 3884 +89
🚀 New features to boost your workflow:
|
szinta
left a comment
There was a problem hiding this comment.
LGTM. Just a few comments related to copyright text and story duplication.
packages/ibm-products/src/components/Tearsheet/Tearsheet.featureflag.stories.jsx
Outdated
Show resolved
Hide resolved
…reflag.stories.jsx Co-authored-by: Sinta Augustine <[email protected]>
|
Are you also planning to implement an equivalent to ComposedModalPresence? While having the tearsheet not mount when it's closed is nice, it alone is not really helping, since the consuming component and all of it's states are still going to remain mounted. With It would be extremely helpful if you could achieve a similar dev experience to what carbon is offering. |
Good point - the limitation you’re calling out is real. We did explore using That said, we agree with the underlying goal. Providing a Tearsheet-specific presence boundary that allows consumers to opt into full lifecycle reset—similar in spirit to @amal-k-joy I'm moving this PR back to draft until the rework is complete. |
@timfuhrmann @makafsal , |
|
I am not sure if making changes to ComposedModalPresence would help, curious to here what you might suggest though. What I was thinking though, that the Tearsheet implements its own Both of these internal carbon utils should be sufficient to achieve a similar outcome:
However, since they are not exposed by carbon yet, we would either have to ask for it or copy/paste. Happy to hear your thought or create a first draft to get started, just let me know. |
|
@timfuhrmann Yeah, those hooks are almost identical, but in |
Closes #8293
What did you change?
Implemented the
enable-presenceand a feature flag to enable it.How did you test and verify your work?
PR Checklist
As the author of this PR, before marking ready for review, confirm you:
More details can be found in the pull request section of
our contributing docs.