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

Replace jquery-ujs with @hotwired/turbo #2448

Merged
merged 1 commit into from
Feb 14, 2024
Merged

Replace jquery-ujs with @hotwired/turbo #2448

merged 1 commit into from
Feb 14, 2024

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented Oct 11, 2023

Replace our client-side dependency on jquery-ujs with
@hotwired/turbo.

In addition to adding an entry to the package.json file, this commit
replaces [data-confirm] attributes with [data-turbo-confirm], and
[data-method] attributes with [data-turbo-method].

In an effort to minimize the diff, this commit does not replace the
link_to calls made with method: :delete with button_to calls.
Ideally, they'd utilize button_to to render a <form> element for the
destructive action.

In the future, once that change is complete, the data: {turbo_method: :delete} attributes can once again be passed as method: :delete
options.

@gillisd
Copy link

gillisd commented Jan 4, 2024

@seanpdoyle Thank you for doing this. What are your thoughts on replacing this window.location = with Turbo.visit as well?

window.location = window.location.protocol + '//' + window.location.host + dataUrl;

Replace our client-side dependency on `jquery-ujs` with
[@hotwired/turbo][].

In addition to adding an entry to the `package.json` file, this commit
replaces `[data-confirm]` attributes with `[data-turbo-confirm]`, and
`[data-method]` attributes with `[data-turbo-method]`.

In an effort to minimize the diff, this commit *does not* replace the
`link_to` calls made with `method: :delete` with [button_to][] calls.
Ideally, they'd utilize `button_to` to render a `<form>` element for the
destructive action.

In the future, once that change is complete, the `data: {turbo_method:
:delete}` attributes can once again be passed as `method: :delete`
options.

[@hotwired/turbo]: https://turbo.hotwired.dev
[turbo-rails]: https://github.com/hotwired/turbo-rails
[link_to]: https://edgeapi.rubyonrails.org/classes/ActionView/Helpers/UrlHelper.html#method-i-link_to
[button_to]: https://edgeapi.rubyonrails.org/classes/ActionView/Helpers/UrlHelper.html#method-i-button_to
@nickcharlton
Copy link
Member

Thanks again for keeping this up to date, @seanpdoyle!

@nickcharlton nickcharlton merged commit 21602c3 into main Feb 14, 2024
10 checks passed
@nickcharlton nickcharlton deleted the turbo branch February 14, 2024 14:12
@seanpdoyle
Copy link
Contributor Author

Thank you for pushing this forward!

Just to highlight some troubles: this PR seems to introduce some test flakiness with regard to the search field. I'm not sure about the cause, since the #2447 PR was the one responsible for modifying how those test drive the search behavior.

It's something to monitor in the future.

@nickcharlton
Copy link
Member

Ah, interesting, thanks! I'll keep an eye on it and open an issue if it keeps happening.

I know on a certain client project we had some trouble with the way we're using to writing feature/system specs and test flakiness and I might need some help digging into it.

nickcharlton added a commit that referenced this pull request Mar 1, 2024
Since we merged in #2448, this has been repeatedly failing on CI. It
doesn't fail locally, which makes it hard to debug.

Fixes #2523
nickcharlton added a commit that referenced this pull request Mar 1, 2024
Since we merged in #2448, this has been repeatedly failing on CI. It
doesn't fail locally, which makes it hard to debug.

Fixes #2523
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

4 participants