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 Turbolinks with Turbo (and upgrade Stimulus) #1181

Merged
merged 26 commits into from
Jan 15, 2024
Merged

Conversation

jim
Copy link
Member

@jim jim commented Nov 8, 2023

What it does

  • Replaces Turbolinks in the app with the newer version of the library, Turbo.
  • Upgrades Stimulus to the latest version
  • Replaces our PortalRendering code with equivalent Turbo-based code.

Fixes #1184.

Why it is important

Turbolinks is EOLed, and we need to upgrade to Turbo to get back on a supported library. And by using Turbo instead of custom code, we have less to maintain and it's easier for contributors to understand how things work as there are many resources out there for learning Turbo.

Implementation notes

  • This is based on the main parts of WIP: Upgrade app to hotwire #810, without the use of real-time Turbo Streams as that requires Redis. We can add that later if needed, but I don't want to pull in that dependency as a part of this.

Your bandwidth for additional changes to this PR

Please choose one of the following to help the project maintainers provide the appropriate level of support:

  • I have the time and interest to make additional changes to this PR based on feedback.
  • I am interested in feedback but don't need to make the changes myself.
  • I don't have time or interest in making additional changes to this work.
  • Other or not sure (please describe):

derricklannaman and others added 6 commits November 17, 2023 08:46
Turbo requires that redirects returned by form submissions have a status of
303 (see other) so that the browser will follow them with a GET request and
not use the HTTP method of the submission request. This is semantically
correct, but a change in behavior from how Rails UJS handled things.

Additionally, Turbo will not render anything when a 200 response is returned
by a form submission, instead requiring that a 422 (unprocessable entity)
status is returned in order for it to render the new HTML and its validation
error messages. This is going to become the Rails default in the future.
There was previously a single controller and set of views that handled
all note CRUD for both Member and Item. As a part of reworking this
code, I created separate controllers and views for the notes on each of
these models. I think this will be eaiser to understand and will allow
the notes placed on each model to diverge more easily.
@jim jim marked this pull request as ready for review December 21, 2023 01:49
@jim jim requested a review from phinze January 15, 2024 17:38
Copy link
Contributor

@phinze phinze left a comment

Choose a reason for hiding this comment

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

LGTM on a basic run through

@jim jim merged commit ddf61a1 into main Jan 15, 2024
5 checks passed
@jim jim deleted the jim/turbolinks-to-turbo branch January 15, 2024 17:40
@jim jim mentioned this pull request Jan 15, 2024
jim added a commit that referenced this pull request Apr 12, 2024
# What it does

Fixes #1438 by moving appointments to the appropriate list when the are
completed or restored. This feature was originally implemented in
dbf9983 but removed in #1181 as a part of the upgrade from Turbolinks to
Turbo.

# Why it is important

Staff have given us feedback that they miss having the app automatically
move completed appointments to the list at the bottom of the page
without needing a page refresh.

# Implementation notes

* The new implementation works on both the new and old UI for the
appointment list.
* `FEATURE_NEW_APPOINTMENTS_PAGE` now defaults to `false` in the test
environment (so we can test that the current default is working in
tests). We could also write tests for both, but that can be a followup.
* I also fixed a bug where completion didn't work properly when
`new=true` using a query param.

---------

Co-authored-by: Paul Hinze <phinze@phinze.com>
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.

Upgrade app from Turbolinks to Turbo
3 participants