[Admin] Avoid javascript in saving positions #10396
Merged
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
Zales0123
added
Enhancement
Tomanhez
reviewed
May 20, 2019
src/Sylius/Bundle/AdminBundle/Resources/views/Product/Grid/Action/updatePositions.html.twig
Outdated
Show resolved
Hide resolved
lchrusciel
reviewed
May 20, 2019
src/Sylius/Bundle/AdminBundle/Resources/views/Product/Grid/Action/updatePositions.html.twig
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/CoreBundle/Controller/ProductTaxonController.php
Outdated
Show resolved
Hide resolved
GSadee
reviewed
May 21, 2019
features/product/managing_products/sorting_products_within_a_taxon_by_position.feature
Outdated
Show resolved
Hide resolved
Zales0123
force-pushed
the
avoid-javascript-in-saving-positions
branch
from
8d42296
to
a9c2498
Compare
Zales0123
force-pushed
the
avoid-javascript-in-saving-positions
branch
from
a9c2498
to
feca621
Compare
kulczy
approved these changes
Jun 19, 2019
pamil
approved these changes
Jun 19, 2019
|
Thank you, Mateusz! 🎉 |
pamil
added a commit
that referenced
this pull request
Aug 8, 2019
This PR was merged into the 1.4-dev branch. Discussion ---------- | Q | A | --------------- | ----- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | | License | MIT Welcome to the continuation of my war against JS scenarios in Sylius ⚔️ :D (see #10396). The idea is the same as previously, to avoid javascript scenarios whenever we can. Currently, each scenario that contains some step with removing an item from the cart needs to be tagged as `@javascript`, as this functionality is done with the ajax call. We can easily avoid it, by setting a proper `form` attribute on each item quantity and coupon fields, as well as update, remove an item and apply coupon buttons. The good thing is - no PHP code is changed 😄 only twigs, js and a new route in yaml. We should also probably deprecate an old `sylius_shop_ajax_cart_item_remove` route (or shouldn't we?). As a result, the build should be at least one minute faster 🚤 (it's a good step forward, anyway 🚀) Commits ------- d9e7f02 Change removing cart item to non-js action 7203a70 Remove @javascript tag from all scenarios that removes cart item
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.
These scenarios were firstly problematic during #10228, so I've fixed it (apparently nope) by adding a focus() method on the element. However, it's still a problem in #10389. The thing is, in the javascript scenarios some other element is covering the "Save position" button if the screen size is too small 🚀 💃
I've realized that we can avoid using an ajax call for saving positions, by using a
formattribute on inputs. If this change will be welcomed, we can do the same for the variants' positions.