chore: add create sidepanel wc pattern code #8379
Conversation
davidmenendez
requested review from
jlongshore and
matthewgallo
and removed request for
a team
✅ 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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8379 +/- ##
=======================================
Coverage 82.01% 82.01%
=======================================
Files 403 403
Lines 18548 18548
Branches 4189 4189
=======================================
Hits 15212 15212
Misses 3336 3336
🚀 New features to boost your workflow:
|
| <c4p-side-panel | ||
| include-overlay | ||
| ?open=${this.open} | ||
| size="md" | ||
| title="Create partitions" | ||
| > |
There was a problem hiding this comment.
So something I noticed in this and in the stepped example, the side panel is controlled by the open state, but we aren't handling the open state updates when a user is closing the side panel either by the keyboard, clicking outside of the sidepanel, or by clicking the "X" close button. What is happening currently appears to require two clicks to re-open the side panel since the first click after the user manually closes the component, the state value is still true, and the button is a toggle so it would on first click, set the state to false (nothing appears to happen for the user), and then the second click would then toggle back to true, showing the side panel... Adding in the event listener and forcing a state change would fix this:
| <c4p-side-panel | |
| include-overlay | |
| ?open=${this.open} | |
| size="md" | |
| title="Create partitions" | |
| > | |
| <c4p-side-panel | |
| include-overlay | |
| ?open=${this.open} | |
| size="md" | |
| title="Create partitions" | |
| @c4p-side-panel-closed=${this._handleClose} | |
| > |
and before the render function, have something like this:
private _handleClose = () => {
this.open = false;
};| <c4p-side-panel | ||
| include-overlay | ||
| ?open=${this.open} | ||
| size="md" | ||
| title="Create partitions" | ||
| > |
There was a problem hiding this comment.
Same comment on the default example.
...ages/ibm-products-web-components/examples/create-side-panel/src/create-side-panel-stepped.ts
Outdated
Show resolved
Hide resolved
davidmenendez
requested review from
amal-k-joy,
anamikaanu96,
makafsal and
szinta
and removed request for
matthewgallo
amal-k-joy
left a comment
amal-k-joy
left a comment
There was a problem hiding this comment.
Everything looks good, 1 minor comment added.
| "dev": "vite" | ||
| }, | ||
| "dependencies": { | ||
| "@carbon/web-components": "^2.37.0", |
There was a problem hiding this comment.
I was wondering if we need to add @carbon/ibm-products-web-components as a dependancy to consume c4p-side-panel ?
szinta
left a comment
szinta
left a comment
There was a problem hiding this comment.
This example looks similar to the c4p-side-panel example with some additional form fields.
Based on the issue description, I assume we need to create a Side Panel example pattern with multiple steps, similar to the Patterns/Step flows/Side panel with steps story in the React Storybook.
Could you confirm if this assumption is correct, or let me know if I’ve missed any key details?
| - [cds-text-input](https://web-components.carbondesignsystem.com/?path=/docs/components-text-input--overview) | ||
|
|
||
| > 💡 Check our | ||
| > [Stackblitz](https://stackblitz.com/github/carbon-design-system/ibm-products/tree/main/packages/ibm-products-web-components/examples/create-side-panel) |
There was a problem hiding this comment.
Stackblitz example is not working as some dependencies are missing in package.json ( @amal-k-joy already added one review comment related to this )
|
@szinta thank you for providing that link! i was very confused about what stepping was mentioned because it's in a different section. i thought perhaps that was referring to something else. idunno... yes i will include that 👍 |
4 participants
Closes #8301
Still a work in progress depending on how granular we want to be with the parity between the web-component and react stories