-
-
Notifications
You must be signed in to change notification settings - Fork 261
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
fix: broken attach and attach another button for searchable associations #3457
fix: broken attach and attach another button for searchable associations #3457
Conversation
Code Climate has analyzed commit 3221d4c and detected 0 issues on this pull request. View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ObiWanKeoni thank you for looking into this and opening this PR!
What happens is that this hidden form field was introduced only for non-searchable fields.
To complete this PR let's remove that hidden field and keep the turbo frame on the url params, or remove it from the params and move the hidden form field outside the if searchable condition ensuring that it always render.
<%# | ||
It is important to render this hidden field to allow selective rendering of the turbo frame. | ||
(e.g. in the case of "Attach and Attach Another") | ||
|
||
Without this, the entire page will be reloaded when the form is submitted. | ||
%> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Paul-Bob I added a comment here for communication purposes. LMK if it should be reworded or if this works. Thank you for your comment - I completely missed the hidden_field_tag
!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thank you @ObiWanKeoni!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this contribution @ObiWanKeoni!
Description
I think this is all that's necessary to fix the issue linked below. In the AssociationsController, we expect a
:turbo_frame
param to be passed to the route if we want to rerender the turbo frame instead of closing it. The only bit I'm unclear on is whether we need to have additional special handling for theattach_another
button. I figured this was enough.Fixes #3456
Checklist:
Screenshots & recording
Manual review steps
Manual reviewer: please leave a comment with output from the test if that's the case.