magento2 icon indicating copy to clipboard operation
magento2 copied to clipboard

feat: implement a standalone cart page

Open sequensucks opened this issue 2 years ago • 4 comments

Description

Implement a standalone cart page. Add a go to cart button in the cart sidebar. Fix useCart linters and cartGetters linters.

Related Issue

re #340

How Has This Been Tested?

Need to test a new page with a route /cart. You can open it from a cart sidebar (go to cart button) or open a link {domain}/cart.

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [x] My code follows the code style of this project.
  • [x] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [x] I have read the CONTRIBUTING document.
  • [ ] I have added tests to cover my changes.
  • [x] All new and existing tests passed.

sequensucks avatar Aug 09 '22 11:08 sequensucks

Hello, @sequensucks, Thank you for your PR. There are general issues that I want to be fixed before we move deeper into the solutions.

  1. We mustn't change the namespace of the cart logic (checkout -> cart module). In Magento, the cart is a part of the checkout module and we want to reflect that, also - the cart is a part of the checkout process so it does make sense to keep it as it was. In addition, such a change will affect everyone and adds not enough value to justify it.
  2. If I understand your change correctly, there will be no sidebar cart, and this is a huge change. We can't make such a shift without thinking of the existing projects. If there should be a standalone cart page then it must be a configurable option such it works as previously unless someone changes the option flag. Then we will avoid breaking change but provide a feature.
  3. Please, use the PR template, put pieces of information into relevant fields, and mark checklist boxes.

bartoszherba avatar Aug 16 '22 06:08 bartoszherba

@bartoszherba thank you for your review. I will fix all general issues

sequensucks avatar Aug 16 '22 06:08 sequensucks

@sequensucks I think that a more proper approach to the cart, instead of a configurable option, would be to do the same as in the M2 Luma theme, there is a cart sidebar, but on the bottom of the sidebar, there is an additional button to go to the cart page.

bartoszherba avatar Aug 16 '22 09:08 bartoszherba

@bartoszherba i restored a cart sidebar and added a go to cart button.

sequensucks avatar Aug 16 '22 11:08 sequensucks

@bartoszherba I will make changes after vacation. From 1.09

sequensucks avatar Aug 23 '22 08:08 sequensucks

@bartoszherba added a changes from the last review, but without unit tests. will add tests after a vacation.

sequensucks avatar Aug 24 '22 13:08 sequensucks

Hello, @sequensucks, Is this the final version ready for a CR iteration?

bartoszherba avatar Aug 30 '22 08:08 bartoszherba

Hello @bartoszherba, No, I fixed the CartSidebar unit tests after adding an useCartView composable. And i want to add unit tests for the useCartView composable. But it will be after 1.09.

sequensucks avatar Aug 30 '22 08:08 sequensucks

Hello @bartoszherba, Sorry for no answer a long time. I checked a PR and i think it is a final version. You can review it. Thank you!

sequensucks avatar Sep 05 '22 07:09 sequensucks

@sequensucks And what about tests we have been talking about recently? Could you add tests for the useCartView composable?

bartoszherba avatar Sep 05 '22 07:09 bartoszherba