Skip to content
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(documentation): migrate collapsible to storybook v7 + lit #1656

Merged
merged 13 commits into from
Aug 3, 2023

Conversation

imagoiq
Copy link
Contributor

@imagoiq imagoiq commented Jul 24, 2023

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Jul 24, 2023

⚠️ No Changeset found

Latest commit: 4a6d9e6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@imagoiq imagoiq linked an issue Jul 24, 2023 that may be closed by this pull request
@swisspost-bot
Copy link
Contributor

swisspost-bot commented Jul 24, 2023

Preview environment ready: https://preview-1656--swisspost-design-system-next-v7.netlify.app

@imagoiq imagoiq added this to the Storybook v7 milestone Jul 25, 2023
Base automatically changed from 1634-migrate-post-icon-to-storybook-v7-lit to main July 25, 2023 14:08
@imagoiq imagoiq force-pushed the 1647-migrate-collapsible-component branch from 25252f7 to 1ac7514 Compare July 26, 2023 09:07
Copy link
Contributor

@alizedebray alizedebray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an issu with the heading-level/headingLevel control:
image

It is probably du to extractArgTypesFactory which has recently been removed from @pxtrn/storybook-addon-docs-stencil :
image

@imagoiq imagoiq requested a review from alizedebray July 27, 2023 07:21
Copy link
Contributor

@alizedebray alizedebray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if we need all the stories here:
image

For me, "Default" and "Custom Trigger" should remain, but the other 3 can be hidden and used only in the documentation as they are a simple variant of the "Default" story.

@imagoiq @oliverschuerch @gfellerph do you have a different opinion?

@gfellerph
Copy link
Member

I am wondering if we need all the stories here: image

For me, "Default" and "Custom Trigger" should remain, but the other 3 can be hidden and used only in the documentation as they are a simple variant of the "Default" story.

@imagoiq @oliverschuerch @gfellerph do you have a different opinion?

@alizedebray we can show a smaller set of stories (or even the docs page only) for components that have only small variations. The header on the other hand profits from multiple stories because you can change the layout dramatically with fullscreen and hidden meta navigation.

Something else @alizedebray, @imagoiq: the styles seem to be off https://www.figma.com/file/xZ0IW0MJO0vnFicmrHiKaY/Components-Post?type=design&node-id=9989-50310&mode=design&t=CY1q4A6PCRSJ9lro-4. Is this intentional?

@imagoiq imagoiq requested a review from alizedebray July 27, 2023 14:33
@alizedebray
Copy link
Contributor

alizedebray commented Jul 28, 2023

Something else @alizedebray, @imagoiq: the styles seem to be off https://www.figma.com/file/xZ0IW0MJO0vnFicmrHiKaY/Components-Post?type=design&node-id=9989-50310&mode=design&t=CY1q4A6PCRSJ9lro-4. Is this intentional?

@gfellerph it is tracked with issue #1291, there is also a PR #1324

@imagoiq
Copy link
Contributor Author

imagoiq commented Jul 31, 2023

Something else @alizedebray, @imagoiq: the styles seem to be off https://www.figma.com/file/xZ0IW0MJO0vnFicmrHiKaY/Components-Post?type=design&node-id=9989-50310&mode=design&t=CY1q4A6PCRSJ9lro-4. Is this intentional?

@gfellerph it is tracked with issue #1291, there is also a PR #1324

Thanks I think that it's ready to be merged then.

@imagoiq imagoiq requested a review from alizedebray August 2, 2023 15:28
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@imagoiq imagoiq merged commit 9b8b574 into main Aug 3, 2023
@imagoiq imagoiq deleted the 1647-migrate-collapsible-component branch August 3, 2023 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation v7: migrate the Collapsible component stories
5 participants