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

Turbo frame refinements #502

Merged
merged 4 commits into from
Sep 26, 2024
Merged

Turbo frame refinements #502

merged 4 commits into from
Sep 26, 2024

Conversation

spohlenz
Copy link
Member

@spohlenz spohlenz commented Sep 24, 2024

The previous iteration of the turbo frame setup, and the attempted fixes in #494, #495 and #497 had some drawbacks still. One of the main ones being that the scopes were outside of the index turbo frame, so scope counts were not being updated when rows were added, deleted or filtered via a search.

This PR:

  • Backs out Combine .app-main and #main turbo-frame #497 and separates .app-main from the #main turbo-frame (it does not feel right for a turbo frame to be a structural element)
  • Moves the #main turbo-frame inside app/views/trestle/application/_layout.html.erb so that it can be bypassed if required.
  • Introduces a new #content turbo-frame within app/views/trestle/application/_layout.html.erb that wraps utilities, tabs and content (but not the flash).
  • Deprecates the #index_turbo_frame helper and removes its usage from the templates. Existing usage should continue to work, essentially functioning as a no-op.
  • Removes the turbo stream layout file, preferring targeted turbo stream actions within each create/update/destroy response.

@coveralls
Copy link

Coverage Status

coverage: 91.41% (-0.07%) from 91.475%
when pulling 04290d2 on turbo-frame-refinements
into c8bf895 on main.

@spohlenz spohlenz merged commit 4da5426 into main Sep 26, 2024
31 of 45 checks passed
@spohlenz spohlenz deleted the turbo-frame-refinements branch September 26, 2024 01:31
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.

2 participants