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

set_turbo_frame_src operation #6

Merged

Conversation

leonvogt
Copy link
Sponsor Contributor

Fixes the current implementation of set_turbo_frame_src for the new operation in: marcoroth/turbo_power#6

Copy link
Owner

@marcoroth marcoroth left a comment

Choose a reason for hiding this comment

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

Hey @leon-vogt, thanks for opening this pull request! Good catch!

Regarding the attribute name: Turbo Stream Actions usually make use of the target attribute for targeting elements. Since we are interacting with <turbo-frame> elements which have regular id attributes we can also rely on the built-in target attribute for that.

This also has the benefit that we can make use of the built-in this.targetElements getter on the client-side if we write the stream element like this:

<turbo-stream action="set_turbo_frame_src" target="frame_one" src="/page2.html"></turbo-stream>

lib/turbo_power/stream_helper.rb Outdated Show resolved Hide resolved
@leonvogt leonvogt force-pushed the add-set_turbo_frame_src-operation branch from fbf8086 to 95634ad Compare October 30, 2022 08:57
@leonvogt
Copy link
Sponsor Contributor Author

@marcoroth thanks for your reply!
Yea makes perfectly sense. Thanks for explaining.
I updated the PR to include your improvement.

Copy link
Owner

@marcoroth marcoroth left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

@marcoroth marcoroth merged commit c1528cc into marcoroth:main Oct 30, 2022
@leonvogt leonvogt deleted the add-set_turbo_frame_src-operation branch October 30, 2022 13:07
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.

None yet

2 participants