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

Generate stories for Phoenix 1.7 core components #187

Closed
zachallaun opened this issue Nov 15, 2022 · 11 comments
Closed

Generate stories for Phoenix 1.7 core components #187

zachallaun opened this issue Nov 15, 2022 · 11 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@zachallaun
Copy link
Contributor

Now that Phoenix 1.7 is in RC, I’m wondering if it would make sense to extend the generator to support automatically generating stories for all CoreComponents that are created for you by default. This would be a really killer feature, both for this project and for Phoenix.

Some things to consider:

  • Should phx.gen.storybook auto-detect the presence of Phoenix 1.7-generated CoreComponents? Or should it be under a flag?
  • Is it worth thinking through how multiple versions of Phoenix-generated components should be supported? E.g. could use something like --core-components-version 1.7 and have it default to whatever is in mix.lock.
  • It’s my understanding that this project intends to move under the Phoenix org and rename to Phoenix.Storybook (Rename the project & repo to phoenix_storybook #80) — not sure if maybe you want to wait for that first? Seems like this could be started in a branch though.

I’d like to contribute if possible and if there’s a good direction/desire to see something like this merged!

@cblavier cblavier added the good first issue Good for newcomers label Nov 16, 2022
@cblavier
Copy link
Contributor

Hey Zach 👋

This would be indeed a great addition.

My opinion:

  • we can set the current phx_live_storybook to support only phoenix 1.7+ on the current main branch
  • people may have deleted the core_components.ex file, or even have deleted some component functions inside, so indeed we can automatically detect the existence of each component before generating the relevant stories

If you want to do it, feel free to open a PR :)

@cblavier cblavier added this to the 0.5.0 milestone Nov 16, 2022
@cblavier cblavier added the enhancement New feature or request label Nov 16, 2022
@zachallaun
Copy link
Contributor Author

Happy to get things started and hopefully see it through!

Any particular organization/implementation opinions? What I’m basically thinking at the moment is:

  • add a core_components directory in priv/templates with a template file per function component. Then the generator can check that the relevant function is exported from an AppWeb.CoreComponents (eg) module before rendering.
  • planning to have a core components page that can explain what is generated and link to applicable Phoenix docs about core components.
  • flag to explicitly set the core components module in case it’s not the standard naming.

@cblavier
Copy link
Contributor

Let's ship it Zach! 🚀

@cblavier
Copy link
Contributor

Hey @zachallaun 👋

Did you already have a chance to take a look at it?
Do you need any help?

@zachallaun
Copy link
Contributor Author

zachallaun commented Nov 30, 2022

Hey @cblavier! I got a basic setup working but then life got in the way, unfortunately, and I haven't had time to revisit it. It's still on my radar, but I'm not sure that it makes sense to block the 0.5.0 release on this!

One option might be to ship this in two steps:

  1. Replace the auto-generated icon storybook with an autogenerated button storybook that comes from your core components (this lets us set up the scaffolding for generated core component stories, basically)
  2. Ship a more complete generator that makes stories for all core components that it can find and ship that in 0.5.1+

Edit: Also, if you or anyone else wants to jump in and just get this done, that's totally reasonable! I don't want to be a blocker for anything.

@cblavier
Copy link
Contributor

cblavier commented Jan 2, 2023

Hey Zach, I'm sorry I didn't get back to you sooner.

I'm planning to work a few days on the storybook this week so I'll make sure to look at what you achieved and give you some feedback

@cblavier
Copy link
Contributor

cblavier commented Jan 3, 2023

@zachallaun, I just had a look at your code, it's neat, thank you 👌
Options 1. and 2. look fine to me.

Do you want to continue working on this? I still have a few tickets that need to get done before shipping 0.5.0
Let me know!

@zachallaun
Copy link
Contributor Author

Great! I’ll circle back around to this — I should be able to at least contribute the scaffolding to ship with 0.5 and then we can work on building out the core component stories in a separate PR to ship later.

@cblavier
Copy link
Contributor

cblavier commented Jan 3, 2023

That's great!
I planned to work this full week on the storybook, so I should reply to you more quickly!

@cblavier
Copy link
Contributor

cblavier commented Jan 9, 2023

Since I just shipped the support for a new kind of story (examples) we will also need to improve generators to generate a full UI example with multiple core components

@cblavier
Copy link
Contributor

cblavier commented Feb 14, 2023

Merged #239
Thanks again, Zach!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants