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

implicit dom_id for morph selectors #436

Merged
merged 15 commits into from
Feb 4, 2021

Conversation

leastbad
Copy link
Contributor

@leastbad leastbad commented Jan 31, 2021

Type of PR (feature, enhancement, bug fix, etc.)

Enhancement

Description

This proposes to allow morph to accept a resource in lieu of a CSS selector. Any resource will be converted to a string with dom_id.

Before:

morph dom_id(post), "I am a teapot"

After:

morph post, "I am a teapot"

Before:

morph {dom_id(post1) => render(post1), dom_id(post2) => render(post2)}

After:

morph {post1 => render(post1), post2 => render(post2)}

I've also made the Reflex#dom_id helper more powerful than its Rails counterpart, so that it now accepts ActiveRecord relations and arbitrary objects (that get converted to strings):

dom_id(post1) # "#post_1" (the current behaviour)
dom_id(Post.all) # "#posts"
dom_id("teapots", "search") # "#teapots_search"

That's all pretty cool, but with the above in place, we can dial it up.

morph User.first

This is equivalent to:

morph dom_id(User.first), render(User.first)

And, thanks to a quirk in the way that morphdom works, this is now possible:

morph Post.all

This is equivalent to:

morph dom_id(Post.all), "<div id=\"#{dom_id("posts")}\">#{ApplicationController.render(Post.all)}</div>"

morphdom DGAF so long as the outermost container element has the same id as the target. If you happen to be rendering a collection with a partial that lines up with render defaults, we can now infer everything but the collection itself.

Finally, I added a render_collection method to Reflex that developers can use for more complex morphs:

render_collection(render(posts, partial: "home/posts", locals: {pagy: pagy}), "#my_content_div")
<div id="my_content_div">post1post2</div>

Thanks to @RolandStuder for the initial inspiration.

Why should this be added

Developer happiness and the principle of least surprise.

Checklist

  • My code follows the style guidelines of this project
  • Checks (StandardRB & Prettier-Standard) are passing
  • This is not a documentation update

Copy link
Contributor

@hopsoft hopsoft left a comment

Choose a reason for hiding this comment

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

I wonder if there is any additional syntactic sugar we could include by leveraging to_partial_path. Too magical?

lib/stimulus_reflex/broadcasters/selector_broadcaster.rb Outdated Show resolved Hide resolved
lib/stimulus_reflex/reflex.rb Outdated Show resolved Hide resolved
lib/stimulus_reflex/reflex.rb Show resolved Hide resolved
@leastbad
Copy link
Contributor Author

leastbad commented Feb 3, 2021

I think that we're implicitly leveraging to_partial_path when we support the morph @posts syntax, even though it's not something we have to do directly.

What do you have in mind? Tell me like I'm not in your head! 🤓

@leastbad leastbad added enhancement New feature or request proposal labels Feb 4, 2021
@leastbad leastbad added this to the 3.4.2 milestone Feb 4, 2021
@leastbad
Copy link
Contributor Author

leastbad commented Feb 4, 2021

I reworked the new dom_id method so that it was a case structure, but standardrb kicked my ass and turned it into an if/elsif and I'm no longer sure it's still cute.

Either way, I had to allow the wrap method to squash the # being prepended to prevent a dreaded double-hash. I also added a sub to the fall-through dom_id case to prevent people from accidentally sending an extra # character into the method.

All of this is open to iteration. 💯

@leastbad leastbad requested a review from hopsoft February 4, 2021 05:39
lib/stimulus_reflex/reflex.rb Outdated Show resolved Hide resolved
@leastbad leastbad merged commit 3ed5fd3 into stimulusreflex:master Feb 4, 2021
@leastbad leastbad deleted the resource_selector branch August 16, 2021 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants