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

[pull] main from solidusio:main #403

Merged
merged 34 commits into from
Mar 21, 2024
Merged

[pull] main from solidusio:main #403

merged 34 commits into from
Mar 21, 2024

Conversation

pull[bot]
Copy link

@pull pull bot commented Mar 13, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

spaghetticode and others added 29 commits March 11, 2024 09:09
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.
TR elements can have only TH and TD descendants, so we made table
rows linkable via Stimulus. Since A tags cannot be used for making
the row linkable, we can't leverage the default Turbo frame
functionality provided via the `data-turbo-frame` attribute.

For this reason, a workaround has been implemented with the previous
commit.
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.
In order to move the promotion controller specs, we need the
controller-specific helpers in our `spec/rails_helper.rb` file.
We want to move some backend functionality here, so we need
solidus_backend.
The routes for the promotion admin are moving into
solidus_legacy_promotions, but they are needed in solidus_admin.
The promotion routes will move to legacy_promotions.
We won't have the admin/promotions route available in backend for much
longer.
Also moves all the promotion-related translations in the `spree`
namespace that are used within these controllers and views.
Note that the promotion rules helper does not live under
app/helpers/spree/admin, but the promotion helper does. Keeping it just
the same.
…-turbo-frames

Convert existing Admin modals to Turbo frames
Move promotion backend controllers and views to legacy_promotions
tvdeyen and others added 4 commits March 18, 2024 08:12
We have to jump through all kinds of hoops to do this otherwise. Just
give the count on hand to the factory.
This spec was flaky, as it generated potentially multiple SKUs per
variant, and it generated them implicitly. These changes makes
everything very explicit, such that we get exactly one stock item per
variant with an identifiable name.
@pull pull bot merged commit 9997bce into nebulab:main Mar 21, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants