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

Simplified live generator #5900

Merged
merged 13 commits into from
Aug 15, 2024
Merged

Simplified live generator #5900

merged 13 commits into from
Aug 15, 2024

Conversation

mcrumm
Copy link
Member

@mcrumm mcrumm commented Aug 14, 2024

Simplifies the phx.gen.live generator with the following changes:

  • Replaces FormComponent with FormLive to handle /new and /:id/edit routes.
  • Removes the :id/show/edit route. FormLive accepts a ?return_to=index|show param to negotiate live navigation after successful form submit.
  • Removes the <.modal> component and its helper functions as they are no longer used.

Copy link
Contributor

@SteffenDE SteffenDE left a comment

Choose a reason for hiding this comment

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

Only one minor comment, lgtm!

lib/mix/tasks/phx.gen.live.ex Show resolved Hide resolved
priv/templates/phx.gen.live/form.ex Outdated Show resolved Hide resolved
@mcrumm
Copy link
Member Author

mcrumm commented Aug 15, 2024

@josevalim ready for another look!

mcrumm added 2 commits August 15, 2024 12:16
- Simplify form template
- Apply action on mount since we don't live navigate within the form live view
- Simplify return_to/back button
Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

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

Beautiful!

@mcrumm mcrumm merged commit 83f3339 into main Aug 15, 2024
14 checks passed
@mcrumm mcrumm deleted the mc-gen-live-no-modal branch August 15, 2024 22:06
@TheArrowsmith
Copy link
Contributor

Why remove modal from corecomponents? Just because it's not used in the generators doesn't mean it's not useful elsewhere, e.g. to include it in one of my own liveviews/components.

@josevalim
Copy link
Member

The rule has always been to ship with the components used by the generators, otherwise we could easily argue that core components should be a 1000LOC file with accordions, stats bars, breadcrumbs, etc. People can always add the features they need.

@josevalim
Copy link
Member

josevalim commented Sep 17, 2024

I would also add that the complexity/size of CoreComponents and the complexity of the generated LiveView pages with LiveComponents and more, have often been cited by newcomers as a challenge for getting started, because it felt too daunting.

At the beginning, we used those generators to showcase LiveView features. The goal with these changes is to align this with the phx.gen.html generators and make them more about providing the simplest foundation for people to build on.

@TheArrowsmith
Copy link
Contributor

Ah okay, that's totally fair. Thanks for the reply!

@clarkware
Copy link
Contributor

Great work, Michael! This will definitely help folks new to Phoenix. Can't wait to have it in an official release! 😀

@kuatroka
Copy link

by newcomers as a challenge for getting started, because it felt too daunting.

My two cents... @josevalim: I would caution you to be very vigilant about the newcomers' complaints. I am a newcomer myself and I came here because I liked what I saw. If you heed everything the newcomers complain about, you are in danger of ruining what the newcomers are actually here for. We might be used to a certain way of doing things that is awful but nevertheless familiar to us, and your way is in reality the proper way. In short, stay strong and implement your vision.

I personally loved modals and, in general, all that was generated by default. I was following the "Programming Phoenix Liveview" book and felt exhilarated when I saw the out-of-the-box, UI and fully functional CRUD capabilities that were already available to me, with all the edit and delete buttons already in place! If I don't like something, I can just delete the part I don't need or comment it out, but if it was not there in the first place, I wouldn't even see how it works.

@josevalim
Copy link
Member

Of course, as any suggestion we receive, from newcomers and experienced developers alike, we discuss, interpret and filter it accordingly. We aren’t doing changes because people ask them. We do them because they ask, we evaluate and then proceed accordingly.

In any case, the CRUD capabilities are all still there. Only the modals are gone.

@kuatroka
Copy link

kuatroka commented Dec 17, 2024

Thanks @josevalim
@mcrumm - would it be possible to share a screenshot of the new UI of what :edit and :show will look like? Thanks

@clarkware
Copy link
Contributor

Is the plan to hold this back until the 1.8 release? I was surprised it wasn't part of recent 1.7 releases.

@josevalim
Copy link
Member

Yes, on v1.8. it would be a breaking change to change the generators this considerably in a patch release. :)

@clarkware
Copy link
Contributor

Thanks. Fair enough. :-)

@TheArrowsmith
Copy link
Contributor

@kuatroka you can see the new UI yourself - just update your mix.exs to get the Phoenix dep from Github main instead of from hex:

{:phoenix, github: "phoenixframework/phoenix"},

You might need to pull some other deps from GitHub too e.g. :phoenix_live_view. I forget the details but it's definitely possible to get Phoenix working locally with latest main to preview the new generators, I've done it myself.

@SteffenDE
Copy link
Contributor

The easiest way to test the new generators is by cloning the repo, fetching deps and then going into the installer folder, from there:

mix phx.new my_app --dev

note the --dev. I think the steps are also written in the README.

@kuatroka
Copy link

kuatroka commented Feb 7, 2025

I've tried @SteffenDE 's approach as I wanted to test in a totally new app before applying it to mine, but faces too many dependency issues when standing up the up and trying to run generators. So I have just left this topic for now. I guess I'll test it when it's all ready and polished. Thanks all

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.

7 participants