Remove "ready for" info from shipping method description area at checkout #12395 (solved) #12443
Conversation
sigmundpetersen
added
the
user facing changes
There was a problem hiding this comment.
Welcome and thank you for contributing on this issue!
I think it needs to change before it's ready, can you please see my comments and make the requested changes?
Additionally, you don't need to merge master in to keep the branch up to date. This causes additional unnecessary commits in the history.
| @@ -93,7 +93,7 @@ | |||
| %em.fees= payment_or_shipping_price(shipping_method, @order) | |||
| - display_ship_address = display_ship_address || (ship_method_is_selected && shipping_method.require_ship_address) | |||
| %div.checkout-input{"data-shippingmethod-target": "shippingMethodDescription", "data-shippingmethodid": shipping_method.id , style: "display: #{ship_method_is_selected ? 'block' : 'none'}" } | |||
| #distributor_address.panel | |||
| -# #distributor_address.panel | |||
| - if shipping_method.description.present? | |||
There was a problem hiding this comment.
We prefer not to keep commented-out code, so you can go ahead and delete the code as per the requirements. If it's required in the future, it shouldn't be hard put back (we can check the git history). Because the codebase is so big, it would be too hard to manage if we kept unused code.
However, from the picture it looks like we want to keep the shipping method description, and only remove the "ready for" section. I think that means removing lines 99-103.
There was a problem hiding this comment.
Will change the code in order to remove only the "ready for" section as noted by dacook.
There was a problem hiding this comment.
Your assumption is correct, i have to delete lines 99-103.
|
There are some failing specs now. Looks like CI is not happy with the indentation :) |
|
Updated the local branch in order to see better why the checks are failing. |
| #distributor_address.panel | ||
| #distributor_address.panel |
There was a problem hiding this comment.
I think you can revert this change. Wouldn't that fix the indentation failures you get?
This provides the solution wanted and the best way to do it.
sigmundpetersen
changed the title
|
Hey @MrBowmanXD , welcome!! I've tested your PR, and looking at the acceptance criteria on #12395, I could verify that, on the -> before this PR
-> after this PR
So, we can see that the Also, I've verified this is appearing on the other pages/places: -> at the top of the checkout pages (verified for all three steps)
-> order confirmation page
-> order confirmation emails
-> on the dropdown of the shop
Looking good! |
What? Why?
sometimes, some shipping method are not available every day. It can raise a conflict with the content the shop manager puts in the "ready for" field. Therefore, we should remove the display of the field in the shipping method description. (according to the creator of the issue)
This change simply comments the code related to the "ready for" info from shipping method description area at checkout/details. Instead of running the code it simply does not execute because it is commented (line 96 until 103 not working).
What should we test?
(these steps were done manually to check)
Release notes
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.