-
Notifications
You must be signed in to change notification settings - Fork 179
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
POC: Editor: Multipage Canvas #11669
Conversation
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.
🐝
Code looks solid. Do agree: there are some pre-existing issues w/ the thumbnail view of pages where the Fit
button overlaps the arrows when there's a lot of pages, but I know that's outside of this scope and body of work.
Are the pages basically all just screenshots? What is the performance impact of this? Can we measure it? It might be best to put this behind a feature flag until that is more clear. |
They are all instances of
That sounds like a good idea! |
Size Change: +727 B (0%) Total Size: 2.64 MB
ℹ️ View Unchanged
|
Plugin builds for ab3bb64 are ready 🛎️!
|
I've pushed a new version with the feature hidden by flag, just FYI. |
Co-authored-by: Pascal Birchler <pascalb@google.com>
# Conflicts: # packages/story-editor/src/components/canvas/layout.js
…-stories-wp into try/9643-multipage
I can reproduce the bug from 00:13 locally and on QA. Screen.Recording.2022-06-22.at.13.58.49.movOther than that we are waiting for the design pod review (page size/margins etc). |
Whoa, that's a weird bug! I'll investigate, thanks for catching it! |
Approved by UX:
To do after UX review:
|
Context
This adds a preview of the pages before and after the current ones to either side of the canvas, if there is enough space. It adds as many pages, as there is room for.
To-do
User-facing changes
It even works in RTL:
Testing Instructions
This PR can be tested by following these steps:
Checklist
Type: XYZ
label to the PRPartially addresses #9643