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(storybook): viewport #5780

Merged
merged 3 commits into from
Sep 12, 2023
Merged

Conversation

AugustinMauroy
Copy link
Member

Description

Adding of viewport plugins to storybook. It's allow us to see how component will render in different device.

Related Issues

No related issues

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npx turbo lint to ensure the code follows the style guide. And run npx turbo lint:fix to fix the style errors if necessary.
  • I have run npx turbo format to ensure the code follows the style guide.
  • I have run npx turbo test to check if all tests are passing.
  • NA I've covered new added functionality with unit tests if necessary.

@vercel
Copy link

vercel bot commented Sep 10, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 10, 2023 11:38am

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

Im not sure if a plugin is needed for something that can easily be done with a browser viewport functionality.

Im usually against adding extra dependencies, and this one doesn't really briny much value, since browsers can do that by themselves.

There's a reason why I removed this plugin to begin with ;)

I'm neutral to be honest, but any extra dependency we always need to ponder if it's worth it. As it's more install time, more transient and peer dependencies, bigger bundle and more chances of having security issues.

@AugustinMauroy
Copy link
Member Author

I agree with you that it's addictive.
The installation time isn't too bad because the project takes 24s to download (with NPM or PNPM).
Why use this package, because it allows you to change the size of the component without touching the gui. In the example below you can see what happens when you use the integrated tool.

Capture d’écran 2023-09-10 à 13 33 10

here's when you use the plugins (it doesn't modify the storybook layout)

Capture d’écran 2023-09-10 à 13 33 43

@bmuenzenmeyer
Copy link
Collaborator

I can see this being useful for upcoming nav bar work. If you feel really strongly @ovflowd that's fine, but honestly I'm surprised this functionality isn't standard storybook behavior. (We built this into Patternlab.io 7+ years ago)

@ovflowd
Copy link
Member

ovflowd commented Sep 11, 2023

I steel feel that the Browser DevTools is sufficient. But I dont have strong feelings, feel free to approve and dismiss my review.

@github-actions
Copy link

Unit Test Coverage Report

Lines Statements Branches Functions
Coverage: 92%
90.24% (185/205) 75% (30/40) 86.66% (39/45)

Unit Test Report

Tests Skipped Failures Errors Time
14 0 💤 0 ❌ 0 🔥 3.235s ⏱️

@ovflowd ovflowd added this pull request to the merge queue Sep 12, 2023
Merged via the queue into nodejs:main with commit 5e51e82 Sep 12, 2023
8 checks passed
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.

3 participants