-
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
Editor: Multipage redesign #11934
Editor: Multipage redesign #11934
Conversation
Size Change: +140 B (0%) Total Size: 2.65 MB
ℹ️ View Unchanged
|
Plugin builds for 1915af6 are ready 🛎️!
|
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.
Works as advertised!
@@ -32,9 +32,9 @@ export const DRAWER_BUTTON_GAP_COLLAPSED = 2; | |||
|
|||
// Thumbnail size varies with available carousel size - over or under this limit | |||
export const WIDE_CAROUSEL_LIMIT = 400; | |||
export const WIDE_THUMBNAIL_WIDTH = 40; | |||
export const WIDE_THUMBNAIL_HEIGHT = 60; | |||
export const WIDE_THUMBNAIL_WIDTH = 45; |
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.
These changes would take effect regardless of the experiment being enabled, that's probably intended?
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.
Yeah, we want to change this regardless.
@swissspidy Looks like this got merged with failing Karma tests and these are now failing on |
Ugh, didn‘t check that. Assumed this was ready since it passed QA. Are they easy to fix? Otherwise let‘s revert |
Not sure since both tests seem to pass locally, for me at least. That doesn't make it very easy -- do these fail locally when you try? |
Is it the Yes it's failing for me locally 😞 Looks like the test is supposed to select both text elements but failing to do so. And because only the Title text element is selected, saving another style doesn't work because it's already been saved. Unfortunately I don't have time right now to debug this more. If nobody else can debug locally, feel free to revert or skip the test & look into it later. |
I'm working on it: #11983 |
Context
This addresses the UX points raised in #11845.
User-facing changes
Context page proportions
Carousel page proportions
Testing Instructions
This PR can be tested by following these steps:
Checklist
Type: XYZ
label to the PRFixes #11845