This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Feature Branch: Updated Shopper Notices #8659
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
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
packages/checkout/components/store-notices-container/index.tsx
packages/checkout/components/store-notices-container/snackbar-notices.tsx |
|
Size Change: -4.31 kB (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
|
I have some tasks around testing but it would be great to get an early review from @woocommerce/rubik before I ask other teams to approve this work. I'll mark the PR as needing review, but please do not merge anything. |
opr
previously requested changes
Mar 9, 2023
There was a problem hiding this comment.
Hey @mikejolley - I did an initial review and tested the functionality, here are some notes:
There are a few TS issues I mentioned inline.
Snackbar List
- Not sure if it's possible, but it would be great to add some actions to storybook to allow adding additional snackbar entries, and also make removing the snackbar items possible, to show off the full functionality.
- Tested in storybook and in the block, also tested with reduced motion, seems to be working OK.
Notice Banner
- Added a few comments for formatting the README, feel free to discard if you don't like the code formatting I suggested.
- Code for component looks good!
- Storybook looks good, though you can't add JSX elements as children via the controls, and when you load the
Error summaryelement, thechildrenprop is red and invalid, i don't think this is too serious though. -
- Tested in the blocks and found an issue when the server returns HTML
-
Store notices container
- You didn't change this line, but line 70 in
packages/checkout/components/store-notices-container/index.tsxcomplains because thegetNoticesaction returnsNotice[]butadditionalNoticesisNoticeType[]so trying to concat the two arrays results in an error, since they apparently don't intersect. There's an issue with thestatustype. Not sure on a fix for now, we could just ignore the error, but I don't feel like we should have to. - Seems to be working fine, as
NoticeBannertested out (mostly) OK in the main notice area and the subcontexts.
packages/checkout/components/store-notices-container/store-notices.tsx
Outdated
Show resolved
Hide resolved
|
I've taken care of that feedback I think @opr. I'll post about this on p2. Cheers. |
0b91d29 to
4accac9
Compare
7 tasks
|
@mikejolley Is this PR ready for review? |
e33a757 to
1bdba0f
Compare
|
@tarunvijwani yes, I just rebased. |
Aljullu
reviewed
Mar 24, 2023
There was a problem hiding this comment.
I really like the new styles, good job! It also works great in light and dark themes. 💯
I did some testing with the Mini Cart and Cart blocks and everything is working great. Leaving some notes below and inline: 👇
- I see
build/blocks-checkout.jsincreased its size by 50%. I guess there is not much that we can do, but wanted to mention just in case. - We should add a changelog entry to the PR.
- It might also be good to add the
Mini Cartlabel to the PR; as the Mini Cart can also display notices.
|
@mikejolley it would be great to include in the docs a couple of images for each notification type so that it's easier for developers to make the connection between them |
|
@ralucaStan Is that not the point of the storybook entries? There you can toggle between the various types. |
* Add templates for legacy core notices * Update src/Domain/Services/Notices.php Co-authored-by: Paulo Arromba <17236129+wavvves@users.noreply.github.com> * Remove debugging code * DRY get_notices_template * Simplify error template * Fix padding * Only include new notices if using block cart/checkout --------- Co-authored-by: Paulo Arromba <17236129+wavvves@users.noreply.github.com>
9964d06 to
b03fa4a
Compare
This was referenced Dec 5, 2023
This was referenced Dec 5, 2023
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 is a feature branch that introduces new components and styling for buyer notices. This includes standard banners, and snackbar notices (which share the same components).
Once in a complete state, we'll reach out to other product teams to ensure this meets our needs internally, and potentially move (or plan to move) these components to a package for wider adoption.
Some brief background; aside from the new look (which we aim to roll out to all notices used on the frontend across WooCommerce), the main change is the move away from
@wordpress/components. This is because the WordPress component library is intended to be used in the editor, not on the frontend of the store. Therefore we're creating new components specifically for that purpose and removing the dependency. Functionally, they are very similar.Todo:
New Components
Note that there are new storybook entries for
NoticeBannerandSnackbarListso feel free to review those there.NoticeBannerDrop in replacement for the
Noticecomponent from@wordpress/componentswith custom icons and new styles.Docs
SnackbarListAccepts an array of notice objects and renders as snackbars in the bottom left corner of the screen. Uses
NoticeBannerfor the notices themselves, but has its own wrapper that handles the styling tweaks and timeout.Docs
Accessibility
prefers-reduced-motionOther Checks
Screenshots
This is from Storybook:
Testing
These new components can be seen in action in Storybook (run
npm run storybooklocally), or using the test cases below.Automated Tests
User Facing Testing
These test cases show how to trigger the new notices during certain activities.
Multiple packages notice
No Shipping Options
No Payment Methods and top level errors
Checkout Validation Errors1. During checkout for a UK address, enter a numeric postcode2. See notice after server refreshCart shipping calculator validation errors
Repeat above test on the shipping calculator on the cart page.
Coupon form snackbars
WooCommerce Visibility
Performance Impact
Dev Note
This update includes new designs/styles for store notices (errors, informational, success) as well as snack-bars, to give a uniform look and feel across all WooCommerce Blocks.
In addition to this, if your store uses either the Cart Block or Checkout Block on the main cart and checkout pages, styles will also transfer to core notices. These new styles should take precedence over any existing theme styles due to the new markup and CSS being applied.
The basic HTML structure of the new notices is as follows:
There are different styles based on the notice status which include
error,success,info,default,warning.If you're not using the Cart or Checkout Blocks (if you're still using the shortcode based cart and checkout for example), existing notices rendered by WooCommerce will not be affected.
Changelog