-
Notifications
You must be signed in to change notification settings - Fork 159
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(sidepanel): introduce side panel component #11201
feat(sidepanel): introduce side panel component #11201
Conversation
Added |
@annawen1 @ariellalgilmore I realise there are some conflicts to resolve, but would love some feedback on the WebComponent SidePanel implementation so far. Just want to make sure I'm not going in completely the wrong direction with my first WebComponent. |
e9c0f6a
to
5279654
Compare
packages/carbon-web-components/src/components/side-panel/side-panel.ts
Outdated
Show resolved
Hide resolved
packages/carbon-web-components/src/components/side-panel/side-panel-header.ts
Outdated
Show resolved
Hide resolved
packages/carbon-web-components/src/components/side-panel/side-panel-header.ts
Outdated
Show resolved
Hide resolved
packages/carbon-web-components/src/components/side-panel/side-panel-story.ts
Outdated
Show resolved
Hide resolved
93676b2
to
2b50f2d
Compare
Deploy preview created for package Built with commit: eb47a754b0a4ee5d4d3a4b0104ab4cd44c92ace5 |
Deploy preview created for package Built with commit: eb47a754b0a4ee5d4d3a4b0104ab4cd44c92ace5 |
Deploy preview created for package Built with commit: eb47a754b0a4ee5d4d3a4b0104ab4cd44c92ace5 |
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.
Left a few comments... the biggest change might be switching from using id=
to class=
packages/carbon-web-components/src/components/side-panel/side-panel-button-set.ts
Outdated
Show resolved
Hide resolved
packages/carbon-web-components/src/components/side-panel/side-panel-story.mdx
Outdated
Show resolved
Hide resolved
packages/carbon-web-components/src/components/side-panel/side-panel.scss
Outdated
Show resolved
Hide resolved
packages/carbon-web-components/src/components/side-panel/side-panel.ts
Outdated
Show resolved
Hide resolved
packages/carbon-web-components/src/components/side-panel/side-panel.ts
Outdated
Show resolved
Hide resolved
packages/carbon-web-components/src/components/side-panel/side-panel.ts
Outdated
Show resolved
Hide resolved
packages/carbon-web-components/src/components/side-panel/side-panel-story.ts
Outdated
Show resolved
Hide resolved
5c340fc
to
a151217
Compare
packages/carbon-web-components/examples/codesandbox/basic/components/side-panel/cdn.html
Outdated
Show resolved
Hide resolved
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 code wise! not sure if we need a design review on this
Should not need a design review, should happen in @carbon/ibm-products for the side panel component. |
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.
I think we can go ahead and merge this in!
c2c5bca
into
carbon-design-system:main
…#11201) Contributes to: carbon-design-system/carbon-labs#16 ### Description Very much a draft version of the SidePanel. It is currently based on the branch `clean_main`. It has a number of missing features Questions: - Button set does not re-order when stacked. I am uncertain if the behavior is possible with Web components - Story knobs not working as expected even when changing stories. Not found a declaration form that is either succinct or correctly updates even between stories. Ideally it should be possible to declare many stories with a single template and different default knobs. Review: - Feedback on the component produced so far would be useful, as it is the fist Web Component I have produced. For feature parity with the React SidePanel the following are required - [ ] button set to re-order when stacked - [ ] Selector-initial-focus not functional - [ ] Add 2nd step story - [ ] Fix action bar refresh bug - this is a circular resize/scroll issue in reality the HTML needs a refactor to correctly use sticky but that involves changing the IBM Products code carbon-design-system/ibm-products#4065 NOTES: - Storybooks feels a bit unwieldy for WebComponents, but that may just be use of V6 storybook.
Contributes to: carbon-design-system/carbon-labs#16
Description
Very much a draft version of the SidePanel.
It is currently based on the branch
clean_main
.It has a number of missing features
Questions:
Review:
For feature parity with the React SidePanel the following are required
but that involves changing the IBM Products code Review Sidepanel title/header DOM structure for web component ibm-products#4065
NOTES: