-
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
Assembler: Add interaction in the Pages screen #83501
Assembler: Add interaction in the Pages screen #83501
Conversation
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~407 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. 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. |
I think the feature flag is not working for me because I don't see the screen when testing on Calypso live: Can you enable the flag in Calypso? wp-calypso/config/wpcalypso.json Line 112 in 68e925b
|
Oops, I had a typo in the description it should be instead of There was a |
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.
The code and functionality are looking good to me 👍
The selected pages are persisted in the URL.
Screen.Recording.2566-10-26.at.12.08.39.mov
Not related to this PR, I'm wondering if we should do something different for existing sites because when they come from the Theme showcase their site can already have these pages. Are we going to replace them or create new ones? cc: @fushar
Great question! I just assumed that we would add those pages and draft existing ones 🤔 |
Good question. I'd just add the pages without worrying about the existing ones. There could be duplicates as a result -- but I think it's clear from the flow, and it's up to the users to choose which pages to keep or trash. 😄 |
{ /* eslint-disable wpcalypso/jsx-gridicon-size */ } | ||
<Gridicon icon="checkmark" size={ 10 } /> | ||
</FlexItem> | ||
<FlexItem className="page-list-item__label">{ label }</FlexItem> |
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.
Just realized that we have to translate the label.
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.
Forgot to mention that this is only "placeholder" UI 😅
We'll have a better handling of data once we integrate the UI with the API!
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.
I see, cool, we can use the pattern or category name because they should be already translated.
Related to https://github.com/Automattic/dotcom-forge/issues/2225
Proposed Changes
This PR introduces the sidebar interaction for the Pages screen.
See the design spec: Tv3pYqA3EcRfiXC31IxrXE-fi-2390%3A25283
Screen.Capture.on.2023-10-26.at.11-47-28.mp4
Tasks for later: API integration to retrieve the list of pages.
Testing Instructions
&flags=pattern-assembler/add-pages
.Pre-merge Checklist