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.

Sign up for GitHub

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

Jump to bottom

[bug] "Remove item" in minicart usability issue #882

Merged
merged 42 commits into from
Mar 21, 2019
Merged

[bug] "Remove item" in minicart usability issue #882

merged 42 commits into from
Mar 21, 2019

Conversation

dmshm
Copy link
Contributor

@dmshm dmshm commented Feb 11, 2019 •
edited
Loading

Description

I used an existing class { LoadingIndicator }, and by using it has blocked the ability to uninstall the product at the time of the request. This adds some convenience on MiniCart.

Related Issue

Closes #661 .

Motivation and Context

This change does not give the user be able to interact with cart items (or the particular items which is being removed) while the request is processing.

How Has This Been Tested?

Steps to reproduce:

  1. User adds 2 or more different items in cart.
  2. For the last item in the list, user clicks "Remove item" from the actions list
  3. The request is being sent in background, this takes some time.
  4. While that is happening in the background, user can still interact with that minicart item, so user clicks on the "Remove item" several more times for the same item. (Decided)
  5. Now all products are removed from the cart, even though user kept clicking "Remove item" for the last item in the list. (Decided)

Screenshots (if appropriate):

Here's how the removal looks like now:
remove_item

Proposed Labels for Change Type/Package

FEATURE

pkg:venia-concept

Checklist:

  • I have read the CONTRIBUTING document.
  • I have linked an issue to this PR.
  • I have indicated the change type and relevant package(s).
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All CI checks are green (linting, build/deploy, etc).
  • At least one core contributor has approved this PR.

@magento-cicd2
Copy link

magento-cicd2 commented Feb 11, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@vercel
Copy link

vercel bot commented Feb 11, 2019 •
edited
Loading

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@vercel vercel bot temporarily deployed to staging February 11, 2019 08:20 Inactive
@vercel vercel bot temporarily deployed to staging February 11, 2019 08:32 Inactive
"Remove Item" in Minicart usability issue #661.
574a742 
Added check for loadingElement object.
@vercel vercel bot temporarily deployed to staging February 11, 2019 17:26 Inactive
d.shmaliuk added 3 commits February 11, 2019 19:34
"Remove Item" in Minicart usability issue #661.
0653e26 
Added check for loadingElement object.
"Remove Item" in Minicart usability issue #661.
9639ac5 
Added check for loadingElement object.
Merge remote-tracking branch 'origin/"Remove_Item"_in_Minicart_usabil…
2e5d876 
…ity_issue_#661' into "Remove_Item"_in_Minicart_usability_issue_#661

# Conflicts:
#	packages/venia-concept/src/components/MiniCart/miniCart.js
#	packages/venia-concept/src/components/MiniCart/product.js
@vercel vercel bot temporarily deployed to staging February 11, 2019 17:38 Inactive
@vercel vercel bot temporarily deployed to staging February 11, 2019 17:43 Inactive
@dmshm dmshm changed the title "remove item" in minicart usability issue #661 "Remove item" in minicart usability issue #661 Feb 11, 2019
@coveralls
Copy link

coveralls commented Feb 11, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 76.917% when pulling c50952e on DmitryShmaliuk:"Remove_Item"in_Minicart_usability_issue#661 into 68cab97 on magento-research:develop.

@vercel vercel bot temporarily deployed to staging February 11, 2019 21:17 Inactive
@supernova-at
Copy link
Contributor

There are some conflicts that need resolved now that #732 has been merged.

Merge branch 'release/2.0' of github.com:magento-research/pwa-studio …
93c8a04 
…into "Remove_Item"_in_Minicart_usability_issue_#661

# Conflicts:
#	packages/venia-concept/src/components/MiniCart/miniCart.js
#	packages/venia-concept/src/components/MiniCart/productList.js
d.shmaliuk added 3 commits February 14, 2019 17:53
Merge branch 'release/2.0' of github.com:magento-research/pwa-studio …
c37e01d 
…into "Remove_Item"_in_Minicart_usability_issue_#661
Merge remote-tracking branch 'origin/"Remove_Item"_in_Minicart_usabil…
763eb96 
…ity_issue_#661' into "Remove_Item"_in_Minicart_usability_issue_#661
Merge branch 'release/2.0' of github.com:magento-research/pwa-studio …
e971b97 
…into "Remove_Item"_in_Minicart_usability_issue_#661
@dmshm
Copy link
Contributor Author

dmshm commented Mar 9, 2019 •
edited
Loading

@sirugh, i like your option. It is much more convenient when one item is blocked, and not absolutely everything.

dmshm
dmshm commented Mar 12, 2019
View reviewed changes
Copy link
Contributor Author

@dmshm dmshm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As i've written, i like this option. It is much more convenient when one item is blocked, and not absolutely everything. The option is easy to understand and solves all the problems with this issue.

packages/venia-concept/src/components/MiniCart/__stories__/Kebab.js Show resolved Hide resolved
@dmshm
Copy link
Contributor Author

dmshm commented Mar 12, 2019

@sirugh, we are satisfied with what we have now? Or have I missed something?

@sirugh sirugh dismissed their stale review March 12, 2019 14:28

I added changes so looking for a different reviewer.

@dmshm dmshm changed the title "Remove item" in minicart usability issue #661 [FEATURE] "Remove item" in minicart usability issue #661 Mar 15, 2019
@dmshm dmshm changed the title [FEATURE] "Remove item" in minicart usability issue #661 [FEATURE] "Remove item" in minicart usability issue Mar 19, 2019
@supernova-at supernova-at self-assigned this Mar 21, 2019
supernova-at
supernova-at requested changes Mar 21, 2019
View reviewed changes
Copy link
Contributor

@supernova-at supernova-at left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some code is missing from MiniCart/miniCart.js.

If no code is missing, this file can be cleaned up a bit.

packages/venia-concept/src/components/MiniCart/miniCart.js Outdated Show resolved Hide resolved
packages/venia-concept/src/components/MiniCart/miniCart.js Outdated Show resolved Hide resolved
packages/venia-concept/src/components/MiniCart/miniCart.js Outdated Show resolved Hide resolved
@dmshm
Copy link
Contributor Author

dmshm commented Mar 21, 2019 •
edited
Loading

@supernova-at please, review my changes.

@supernova-at supernova-at merged commit 4b82402 into magento:develop Mar 21, 2019
@sirugh sirugh added the version: Patch This changeset includes backwards compatible bug fixes. label May 24, 2019
@sirugh sirugh changed the title [FEATURE] "Remove item" in minicart usability issue [bug] "Remove item" in minicart usability issue May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sirugh sirugh sirugh left review comments

@supernova-at supernova-at supernova-at approved these changes

Assignees

@supernova-at supernova-at

Labels
version: Patch This changeset includes backwards compatible bug fixes.
Projects
None yet
Milestone
Next PWA Release (Core 2.3.2)
Development

Successfully merging this pull request may close these issues.

"Remove Item" in Minicart usability issue
9 participants
@dmshm @magento-cicd2 @coveralls @supernova-at @soumya-ashok @sirugh @cherdman @jcalcaben @dani97