Improve coupon code JS applicator #1090
Conversation
| coupon_code = $.trim(coupon_code_field.val()) | ||
| return if coupon_code == '' | ||
|
|
||
| if $('#coupon_status').length == 0 |
There was a problem hiding this comment.
Is this necessary? The class is defined in the view, we should be able to rely on that.
There was a problem hiding this comment.
I agree. Took it from the old script, sort of backwards view compatible, but I guess given that the view needs a button now, I can get rid of this.
|
Thanks for reviewing @jhawthorn ! I'd be happy to also hear any other comments if anyone has. |
|
This looks pretty good to me, thanks @mtomov, 👍. And I have nothing valuable to add to the open conversations. |
|
I'm just very happy about the |
|
Hi @jhawthorn , I've made this one change in the JS (don't auto-create the div tag). It's good that we don't attach to the Let me know if there's more I can adjust! |
|
👍 |
|
Still good by me, nice work 👍 We're almost definitely going to need a changelog entry for this change though. |
| </p> | ||
| <div id='coupon_status' | ||
| data-success-class="success" |
There was a problem hiding this comment.
Since these values match the defaults here https://github.com/solidusio/solidus/pull/1090/files#diff-a065997c80be8446e236252c98a12800R7 we don't need to duplicate them here.
There was a problem hiding this comment.
Yes, but they are present for documentation purposes here. Alternatively, I can remove the defaults in the JS, but I thought they don't add much noise, so why not have them.
There was a problem hiding this comment.
They imply that they are overwriting default behaviour. If we want to document it both the changelog and the JS file make very good candidates for that.
There was a problem hiding this comment.
Yeah, but frontend developers (and even lazy people like me) don't tend to meddle with the middle of some JavaScript file to find out that they can actually customize the classes.
- separate Coupon code application from Saving & processing a payment
=> allows users to first apply a code, and subsequently add payment
details when they are certain of the final amount to be paid.
- Add new button Apply Code
- Separate payment js from coupon code js - allows for easier override
as they are now in separate files.
- Allow customisation of the CSS classes applied on success and on error
via data attributes `data-success-class` and `data-success-error`.
- Fix a refresh bug on coupon code application error - before, the page
was refreshed even after displaying an AJAX brought error message.
See issue & fix @spree spree/spree#7040
- Fixes a JS warning that a Synchronous AJAX request was executed, which
could impact JS performance as it waits for response from the server
before continuing the execution of the JS.
(Was coming from the `async:false` setting in `Spree.ajax`)
- Switch to using the Spree API action for applying a code instead of the
Frontend controller one at StoreController#apply_code.
TODO: Delete the StoreController#apply_code similarly to Spree as
it's not used anymore. See: spree/spree#7284
|
Changelog added + CSS classes streamlined. Let me know of anything else. Otherwise I can squash it. Thank you for the feedback! |
- Specify your own class as a main one, ex: `callout`
- The JS applicator will automatically add `success` or `alert` classes
as appropriate
|
👍 Thank you |
|
👍 |
Applying coupon code should be only handled with its own "Apply Code" button, which will trigger an API call that tries to apply the coupon code and refresh the page (solidusio#1090).
Separate Coupon code application from Saving & processing a payment
» allows users to first apply a code, and subsequently add payment details when they are certain of the final amount to be paid.
Add new button Apply Code
Separate payment js from coupon code js - allows for easier override
as they are now in separate files.
Specify your own css class as a main one, ex:
callout. The JS applicator will automatically addsuccessoralertclasses as appropriate.Allow customisation of the CSS classes applied on success and on error
via data attributes
data-success-classanddata-success-error.Fix a refresh bug on coupon code application error - before, the page
was refreshed even after displaying an AJAX brought error message.
See issue & fix @spree do not submit form if coupon code is invalid at payment step spree/spree#7040
Fixes a JS warning that a Synchronous AJAX request was executed, which could impact JS performance as it waits for response from the server before continuing the execution of the JS. (Was coming from the
async:falsesetting)Switch to using the Spree API action for applying a code instead of the
Frontend controller one at StoreController#apply_code.
TODO: Delete the StoreController#apply_code similarly to Spree as
it's not used anymore. See: Removing Spree::StoreController#apply_coupon_code spree/spree#7284