Convert existing modals to use turbo frames #401
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, all flash messages were showing with the same (black) background. Internally, the flash object converts and stores keys as strings [1]. [1] https://github.com/rails/rails/blob/main/actionpack/lib/action_dispatch/middleware/flash.rb#L163
spaghetticode
force-pushed
the
spaghetticode/admin-new-tax-category
branch
from
efc79a7
to
5584f93
Compare
spaghetticode
force-pushed
the
spaghetticode/convert-modals-to-turbo-frames
branch
from
5425f5b
to
edef9ab
Compare
…n-flash [ADMIN] Fix flash messages coloring
…w-tax-category [Admin] Create new Tax Categories
We can remove all this custom behavior since we want the modal to be rendered async on top of the current page.
This way, the modal can be loaded async into the current page. Please note that the order page rendering, which is discarded on turbo-frame calls, needs to be moved to the bottom, as it includes an empty turbo frame tag with same ID that would interfere with the modal rendering.
The `respond_to` block is required to make Turbo frames and streams work. With this change, the order edit modal can be rendered async on top of the calling page (order edit). Form submission, when successful, triggers a turbo stream page refresh, updating contents (namely the order email) while preserving the scroll position.
URLs for editing addresses are handled via turbo frames, enclosing the modals inside their appropriate turbo frame tag addded on the page.
Just like we did a few comments before with the order email modal.
`Turbo.visit` already supports turbo frames, so we just need a way to pass the information from the table HTML to the `visit` call. This implementation, although arbitrary and possibly questionable, has the merit of being simple, meaning that enabling turbo frames in a table row requires just to update the URL to include the `_turbo_frame` param. This param is removed from the URL, so it won't have any possible further side effect in the application.
This leverages the new functionality from the previous commit. Also, adds the turbo_frames list to be injected into the component view.
Since the modal open and closes on the current page, close links are not needed anymore. Same goes for the explicit q and page params. The `Cancel` button is simplified to use dialog standards.
Separated from the previous commit for easing reviews.
With the modal managed via turbo frame and turbo stream refresh, all this custom explicit params management is not needed anymore.
spaghetticode
force-pushed
the
spaghetticode/convert-modals-to-turbo-frames
branch
from
edef9ab
to
b3b0e2c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This complements solidusio#5674 by converting previously existing modals to turbo frame/turbo stream page refresh.
Unfortunately, this PR cannot be opened in the main Solidus GH space, as the first PR is coming from Nebulab, so this PR aims to preview the changes to be done to the existing modals.