This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Remove old cart notices before showing new ones #7363
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
opr
added
status: needs review
type: bug
rubikuserbot
requested review from
a team and
nielslange
and removed request for
a team
|
The release ZIP for this PR is accessible via: |
|
Size Change: +156 B (0%) Total Size: 1.02 MB
ℹ️ View Unchanged
|
nielslange
approved these changes
Oct 12, 2022
github-actions
bot
added
status: ready to merge
and removed
status: needs review
labels
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
nielslange
removed
the
status: stale
|
cc @nielslange, @jimjasson and @PanosSynetos @manospsyx - I am going to close this PR without merging until we get further progress and a decision on pca54o-4vF-p2. I feel the new UX here, where old notices get removed before the shopper has a chance to see them is too poor. Let me know if you really disagree and we can look into reopening it. |
|
Hey @opr, thank you for the update! My only concern is that this issue is a blocker for peapX7-KN-p2. |
|
@opr what you describe pca54o-4vF-p2 in your example is very much of an edge case. Let me give a more “sensible” example, with a screenshot (coming straight from MMQ)
When the user adjusts the quantity and sets it to 5, the error message should go away, shouldn’t it? Well... it doesn't ;) I'm afraid we cannot default the cart/checkout blocks on wpcom/ecommerce plans without this fix, MMQ is one of the crucial plugins of this plan |
Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the |
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
nielslange
removed
the
status: stale
opr
force-pushed
the
fix/remove-errors-when-fixed
branch
from
5c9515a to
8a244d9
Compare
TypeScript Errors ReportFiles with errors: 439 🎉 🎉 This PR does not introduce new TS errors. |
11 tasks
opr
force-pushed
the
fix/remove-errors-when-fixed
branch
from
18554d5 to
bcc9f03
Compare
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
opr
force-pushed
the
fix/remove-errors-when-fixed
branch
from
bcc9f03 to
0d27967
Compare
|
The release ZIP for this PR is accessible via: |
|
Hey @PanosSynetos @jimjasson @mikejolley would you be able to try this one out again? I have rebased on Zip of changes: |
github-actions
bot
removed
the
status: stale
opr
force-pushed
the
fix/remove-errors-when-fixed
branch
from
0d27967 to
6f86b6f
Compare
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
|
Hey @opr 👋 I tested the zip file you shared and confirmed that errors get hidden when they are no longer valid. Moreover, I confirmed that the errors show up again if the user makes an invalid selection. Here's a video of what I tried: Screen.Capture.on.2023-01-10.at.17-00-48.mp4In my example, the Minimum Order Quantity is 5 and the Maximum Order Quantity is 10. |
opr
force-pushed
the
fix/remove-errors-when-fixed
branch
from
6f86b6f to
51aee45
Compare
|
Sorry @opr - I missed your message :( @jimjasson was your test results ok? Do you need me to test it out on my end? It might be a good idea to try it out on Atomic and see how it behaves |
|
All good on a local site @PanosSynetos -- feel free to give this a try on an Atomic site if you have one available! |
|
I tested with https://wcblocks.wpcomstaging.com/wp-content/uploads/woocommerce-gutenberg-products-block-7363.zip on an ecommerce Atomic site and works as expected in Cart with But, the same changes don't reflect into Mini Cart 🙈 I get an error that I need at least 5 items, but after I update the quanity, the error doesn't go away. If I reload and open the minicart, the error is gone Screencast is with the Tsubaki theme , on an ecommerce plan site Monosnap.screencast.2023-01-11.11-37-26.mp4 |
opr
force-pushed
the
fix/remove-errors-when-fixed
branch
from
51aee45 to
8fe3226
Compare
|
@PanosSynetos please could you test again with the latest zip with the mini cart fixed 🤞🏼 |
|
All is good now, I tested both the cart and mini cart! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR will change the
Filled Cart Blockso that it will clear out previous errors before showing new ones. This is because in #5838 we noticed that even though old errors had been fixed and were no longer present in the API response, they were not removed from the UI.Fixes #5838
Testing
Automated Tests
User Facing Testing
WooCommerce Visibility
Performance Impact
This will have a very slight, pretty much negligible, negative performance impact after receiving the cart from the API. This change will cause the Cart block to dispatch n actions to the
core/noticesstore, where n is the number of notices currently displayed every time the cart updates. I am noting this anyway just so it's on the record, despite it being negligible.Changelog