Skip to content

Storybook Tool#1

Merged
puntope merged 22 commits intotrunkfrom
add/storybook
May 20, 2022
Merged

Storybook Tool#1
puntope merged 22 commits intotrunkfrom
add/storybook

Conversation

@puntope
Copy link
Contributor

@puntope puntope commented May 19, 2022

Changes proposed in this Pull Request:

Initial PR adding Project Templates as well as Storybook implementation

Detailed test instructions:

  1. Uninstall all the storybook dependencies in GLA
  2. Install this branch in the project using npm i -D 'https://gitpkg.now.sh/woocommerce/grow/storybook?add/storybook'
  3. Check in node_modules you have woocommerce-grow-story-book folder
  4. Check in node_modules/bin you have storybook.js
  5. Replace storybook lines in scripts with this in GLA package JSON
"storybook": "storybook",
"storybook:build": "storybook build",
"storybook:deploy": "storybook deploy",
  1. Execute npm tun storybook and see how the storybook watch initialises and you can access the URL
  2. Execute npm tun storybook:build and see how the storybook generates dist directory
  3. Execute npm tun storybook:deploy and see how the storybook deploys the dist version to GH pages in https://woocommerce.github.io/google-listings-and-ads/?path=/story/tree-select-control--base
  4. Check the templates generated with yo:ventures

Additional details:

Changelog entry

Add - Storybook
Add - Repository Templates

@puntope puntope requested a review from a team May 19, 2022 13:28
@puntope puntope self-assigned this May 19, 2022
@tomalec
Copy link
Contributor

tomalec commented May 19, 2022

💅 I'm currently baking a PR to add Yeoman generator here would you mind skipping the first commit 50f7260
So adding a generator will use the local/updated generator?
It's not a big deal, I can update it after this one, this is just my OCD of keeping history cleaner, as I'd change the dotfiles content to use the project name to
🚧 "Ventures Grow Packages"

❓ Also I was thinking of adding individual packages to /packages/js/storybook /packages/php/something_for_composer_to_use to follow woocommerce/woocommerce example https://github.com/woocommerce/woocommerce/tree/trunk/packages, WDYT?

@tomalec
Copy link
Contributor

tomalec commented May 19, 2022

💅 Have you considered ES Modules instead of CommonJS syntax? https://github.com/woocommerce/grow/compare/add/storybook...add/storybook-esm?expand=1
So eventually some day, we could forget about legacy stuff and heavy transpiling ;)

Copy link
Contributor

@tomalec tomalec left a comment

Choose a reason for hiding this comment

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

Tested locally. Works nice. I left few cosmetic 💅 comments. The only blocking 🚧 one is about "Ventures Grow Packages" name, plus a few questions ❓ .

💅 Also, using this package as a dependency removes coloring from the console :(
is there something easy we can do about it?
image

@tomalec
Copy link
Contributor

tomalec commented May 19, 2022

I did a quick check and the shelljs-live proposed in shelljs/shelljs#86 (comment) seems fine. The question is would be like to rely on such unpopular package.

@puntope
Copy link
Contributor Author

puntope commented May 20, 2022

about legacy stuff and heavy transpiling ;)

What do you mean, do you have some examples about this?

I changed it tho in last commits in advance.

@puntope
Copy link
Contributor Author

puntope commented May 20, 2022

💅 I'm currently baking #2 to add Yeoman generator here would you mind skipping the first commit 50f7260

No problem. I removed the files referent to that commit.

@puntope
Copy link
Contributor Author

puntope commented May 20, 2022

💅 Also, using this package as a dependency removes coloring from the console :(
is there something easy we can do about it?

I did a quick check and the shelljs-live proposed in shelljs/shelljs#86 (comment) seems fine. The question is would be like to rely on such unpopular package.

For now i'd prefer to keep it without fancy colors. Since my goal is to have this done in current cooldown, getting rid of Storybook deps in GLA. But definitely I'm down for experimenting with some tools to improve the experience in a future PR.

@puntope
Copy link
Contributor Author

puntope commented May 20, 2022

Also I was thinking of adding individual packages to /packages/js/storybook /packages/php/something_for_composer_to_use to follow woocommerce/woocommerce example https://github.com/woocommerce/woocommerce/tree/trunk/packages, WDYT?

Sure no problem, I changed that in last commits. 👌

@puntope
Copy link
Contributor Author

puntope commented May 20, 2022

Howdy @tomalec Thanks for the suggestions.

This is ready for another review 👌

@puntope puntope requested a review from tomalec May 20, 2022 08:21
puntope and others added 2 commits May 20, 2022 16:31
Co-authored-by: Tomek Wytrębowicz <tomalecpub@gmail.com>
Co-authored-by: Tomek Wytrębowicz <tomalecpub@gmail.com>
puntope and others added 2 commits May 20, 2022 16:33
Co-authored-by: Tomek Wytrębowicz <tomalecpub@gmail.com>
Co-authored-by: Tomek Wytrębowicz <tomalecpub@gmail.com>
Copy link
Contributor

@tomalec tomalec left a comment

Choose a reason for hiding this comment

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

Tested locally, reviewed the code LGTM

@puntope puntope merged commit 0770268 into trunk May 20, 2022
@puntope puntope deleted the add/storybook branch May 20, 2022 14:55
ibndawood added a commit that referenced this pull request Aug 28, 2023
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.

2 participants