-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
Site Assembler - Snackbar notifications as in the editor #74813
Site Assembler - Snackbar notifications as in the editor #74813
Conversation
I'll add more details in the description, a video and more testing steps soon. |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~630 bytes added 📈 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~2121 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~627 bytes added 📈 [gzipped])
React components that are loaded lazily, when a certain part of UI is displayed for the first time. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
...ng/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx
Show resolved
Hide resolved
...g/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.scss
Show resolved
Hide resolved
...g/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.scss
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some questions about SASS and testing.
@Automattic/lego I added a video and more details in the description. It's ready for review! |
...ng/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx
Outdated
Show resolved
Hide resolved
...ng/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx
Outdated
Show resolved
Hide resolved
...ng/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx
Outdated
Show resolved
Hide resolved
...g/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.scss
Outdated
Show resolved
Hide resolved
client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/index.tsx
Show resolved
Hide resolved
client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/index.tsx
Show resolved
Hide resolved
@davewhitley @Automattic/lego Initially, I wasn't going to include the notice when a section pattern is removed but I decided to include it for testing. Also, for the same reason, this PR shows a notice when the header or footer pattern is added, replaced, or removed. While using the category name makes sense for section patterns because it's the name shown in the list, in the case of the header and footer patterns, using the category name For headers and footers, we could add an exception to use the singular in the notice Another idea is adding The editor uses the pattern name in the notice, we could use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Should we use "inserted" here instead instead of "added"? |
Yes, I'll follow up with other PR to update that copy in all the areas. |
@miksansegundo Some questions/comments:
Regardless, once we replace the text list of sections patterns with a thumbnail list, then the snackbars should use the pattern name as we are not constricted by space. |
c1b2c7d
to
6819b4d
Compare
@autumnfjeld thanks for the feedback. I've updated the copy to use the same convention already used in the editor: |
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/7884599 Hi @miksansegundo, could you please edit the description of this PR and add a screenshot for our translators? Ideally it'd include all of the following strings:
Thank you in advance! |
Translation for this Pull Request has now been finished. |
Related to #74758
Proposed Changes
pattern-assembler/notices
withNotices
HOC to handle the notices stateSnackbarList
component to handle the stack of notices.Block pattern "Name" inserted.
Screen.Recording.2566-03-24.at.14.19.54.mov
Testing Instructions
Calypso Live
link from the first commentStart designing
Homepage > Add patterns
to add patterns and verify that the notice shows the category name of the patterns addedreplaced
.removed
<
back orSave
button and verify the notices stayContinue
to verify the notices disappearPre-merge Checklist