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

chainable selectors #107

Merged
merged 7 commits into from
Apr 9, 2021
Merged

chainable selectors #107

merged 7 commits into from
Apr 9, 2021

Conversation

leastbad
Copy link
Contributor

@leastbad leastbad commented Feb 6, 2021

This PR proposes 4 related enhancements that brings some recent changes in StimulusReflex full circle.

  1. The dom_id function has been moved to CableReady::Identifiable, which we will include in StimulusReflex's reflex.rb class as a way to consolidate one implementation of the function. Also, users can include this module in their own projects.
  2. CableReady selectors can now accept ActiveRecord::Base and ActiveRecord::Relation objects, the same way the new StimulusReflex morph method accepts them eg. implicit dom_id for morph selectors stimulus_reflex#436
  3. When adding an operation, the method now accepts an optional selector as a first parameter. This is a shortcut since so many operations accept a selector, and saves the developer from having to type selector: "#users". If a selector is provided in the options, it overrides a specified selector.
  4. Each CableReady channel now remembers the selector from the previous operation, if any. This means that you can specify a selector at the beginning of a chain, and it will automatically be picked up by succeeding operations until a new selector is used, at which point that becomes the "new previous" operation for all following operations; if a new selector is used, all previously used selectors are unmodifed.
include CableReady::Identifiable

puts dom_id(User.all, :happy) # "#happy_users"
puts dom_id(User.first) # "#user_1"

cable_ready.inner_html(selector: User.first, html: "<span>Your mother</span>")

cable_ready.inner_html("#smelly", html: "<b>All hype</b>")
cable_ready.inner_html(User.all, html: "<i>mmm</i>")

cable_ready.set_focus("#smelly").inner_html(html: "<span>I rock</span>").set_style(name: "color", value: "red").text_content(selector: User.all, text: "Bloom")

@leastbad leastbad added enhancement proposal ruby Pull requests that update Ruby code labels Feb 6, 2021
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.

This is a nice improvement. Love seeing those tests.

@leastbad
Copy link
Contributor Author

leastbad commented Apr 8, 2021

I'm still fuzzy on requires. Wouldn't it make sense to have it in the top-level file and remove it from broadcaster?

@marcoroth
Copy link
Member

I'd vote to have the requires top-level in lib/cable_ready.rb.

Maybe it also makes sense to switch to Zeitwerk in the future.

@leastbad
Copy link
Contributor Author

leastbad commented Apr 9, 2021

I have moved all of the requires in the project to lib/cable_ready.rb and it seems to work well in my tests.

Generally speaking, I don't really understand what would motivate require_relative or requiring outside of the top-level file. If you know that you're going to need it, why not keep it all in one place? This is not a rhetorical question! If there's a nuance I don't understand, I'm eager to learn.

I don't want to merge until someone either concurs or gently informs me that I was a little too aggressive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement proposal ruby Pull requests that update Ruby code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants