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

Add doc examples using attach_hook for code organization instead of LiveComponents #3685

Merged

Conversation

ericridgeway
Copy link
Contributor

Update Welcome doc to use the guidance in "Functional components or live components?"
(near the top of Phoenix.LiveComponent) to avoid using LiveComponents
only for organization

Add hierarchy headers to the "Compartmentalize state, markup, and events" section

Provide some example code with a few different alternatives of what to use for
organizing handle_events other than LiveComponents

Example code adapted from @Gazler's incredibly helpful explanation in the below Issue

Resolves #3679

Add small hierarchy headers to the "Compartmentalize state,
markup, and events" section

Use the guidance in "Functional components or live components?"
at the top of `Phoenix.LiveComponent` to avoid using LiveComponents
only for organization

Provide longer example with a few different alternatives of
what *can* be used for organizing events
Copy link
Member

@Gazler Gazler left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, this is a really good start to a practical example of when to use attach_hook/4

I've left a few comments that I think will make it a little clearer, once they're done, I think this will be good to merge.

@ericridgeway
Copy link
Contributor Author

Ok, I think that resolves the 3 helpful comments, ty for the tips!

@ericridgeway ericridgeway requested a review from Gazler February 20, 2025 13:42
@SteffenDE
Copy link
Collaborator

woops, should have used that prominent "add suggestion to batch button"

@SteffenDE SteffenDE requested a review from josevalim February 20, 2025 17:36
@SteffenDE
Copy link
Collaborator

I know that José loves good documentation, so let's get his eyes on it as well :)

@josevalim
Copy link
Member

This is a great contribution but it is too much detail for the welcome.md page. I think we should break this into its own guide and make the welcome page be a subtle introduction into these topics.

Now the "using attach_hook/4 to extract handle_events" example
code is actually kept in the doc FOR attach_hook

This keeps the welcome.md page similar to it's original size
@ericridgeway
Copy link
Contributor Author

That's a fair point @josevalim!

Before I had the long code example in welcome.md, and a link to that example in attach_hook/4's doc

I've reversed those. Now the "attach_hook/4 to extract handle_events" example is actually kept in the doc FOR attach_hook/4

This keeps the welcome.md page similar to it's original size

@ericridgeway
Copy link
Contributor Author

ericridgeway commented Feb 21, 2025

Option B:
We can pull the "Compartmentalize state, markup, and events in LiveView" section out of introduction/welcome.md and into a new server/code_organization.md guide file

Do you have a preference between Option A (the most recent commit) vs B?

@josevalim
Copy link
Member

I like this new direction! My suggestion though, instead of rocking the boat too much, is to simply mention attach_hook/3 within the function components part. Something like: "Function components could also be paired with attach_hook/3, in case you want to use the same event handler in multiple places". WDYT?

@ericridgeway
Copy link
Contributor Author

I like this new direction! My suggestion though, instead of rocking the boat too much, is to simply mention attach_hook/3 within the function components part. Something like: "Function components could also be paired with attach_hook/3, in case you want to use the same event handler in multiple places". WDYT?

So merging the "attach_hook/4 to organize event handling" section into the "Function components" one above it?

I like it!

@josevalim
Copy link
Member

Yes, let's keep the change to the welcome page minimal, basically adding a sentence or two, and then focus on adding more examples to attach_hook!

- Merge the "attach_hook/4 to organize event handling" section into
the "Function components" one above it

- Return to the previous bullet-point based summary, adding only a single
line to emphasize that function components can work alone for markup, but
also organize event handling when paired with attach_hook/4
@ericridgeway
Copy link
Contributor Author

ericridgeway commented Feb 21, 2025

@josevalim All set! I've tried to minimize changes to welcome.md. How do we feel about this version?

I do believe many of us struggled with the question of:
"I can use function components to pull html into another module, but how do I also extract the handle_events called-by & highly-related-to that html I just pulled out?"

For lack of a (known) better option, multiple tutorials and recently-published books still reach for LiveComponents to achieve that.
In fact, the currently-live doc in this very section of welcome.md still suggests that, despite the admonition in Phoenix.LiveComponent

So I wanted to at least slightly emphasize that there IS an existing method to organize event handling in another module without resorting to LiveComponents when not needing the separate state

@ericridgeway
Copy link
Contributor Author

Thank you for the suggestions and fixes @josevalim! I think these examples and nudges towards attach_hook will help other beginners like me a lot

And thank you again to @Gazler. Both for the awesome example code, and for showing me phoenix_playground. Love it! ❤️

Were there any other tweaks or is this PR ready to be merged?

@SteffenDE SteffenDE merged commit 573141b into phoenixframework:main Feb 23, 2025
7 checks passed
@SteffenDE
Copy link
Collaborator

Thank you a lot! 🙌🏻

SteffenDE added a commit that referenced this pull request Feb 23, 2025
…iveComponents (#3685)

* Update attach_hook/4 with extract handle_event example

* Add small header for doc linking

* Clarify the code-organization sections of the Welcome guide

Add small hierarchy headers to the "Compartmentalize state,
markup, and events" section

Use the guidance in "Functional components or live components?"
at the top of `Phoenix.LiveComponent` to avoid using LiveComponents
only for organization

Provide longer example with a few different alternatives of
what *can* be used for organizing events

* Remove suggested option to delegate events manually

As pointed out, sticking with attach_hook keeps ownership of
the event in the new component

* Remove Helpers and use more-standard {:ok, socket} style

* Add forgotten line break

* Remove 2nd attach_hook example and link directly to the first

* Update lib/phoenix_live_view.ex

* Update guides/introduction/welcome.md

* Update guides/introduction/welcome.md

* Update guides/introduction/welcome.md

* Update guides/introduction/welcome.md

* Update guides/introduction/welcome.md

* Update guides/introduction/welcome.md

* Update guides/introduction/welcome.md

* Reverse locations of long-code-example & link-to-that-example

Now the "using attach_hook/4 to extract handle_events" example
code is actually kept in the doc FOR attach_hook

This keeps the welcome.md page similar to it's original size

* Rename "Organizing code" section -> "Sharing event handling logic"

* Continue minimizing changes to welcome.md

- Merge the "attach_hook/4 to organize event handling" section into
the "Function components" one above it

- Return to the previous bullet-point based summary, adding only a single
line to emphasize that function components can work alone for markup, but
also organize event handling when paired with attach_hook/4

* Apply suggestions from code review

* Apply suggestions from code review

---------

Co-authored-by: Gary Rennie <[email protected]>
Co-authored-by: Steffen Deusch <[email protected]>
Co-authored-by: José Valim <[email protected]>
@ericridgeway ericridgeway deleted the attach-hook-for-code-organization branch February 23, 2025 20:17
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.

Unclear documentation for code organization (such as extracting related handle_events to a new module)
4 participants