Skip to content
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

FSE NUX: fix the dimensions of the preview image #42675

Merged
merged 2 commits into from
May 28, 2020

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented May 26, 2020

Changes proposed in this Pull Request

To boldly fix the dimensions of the preview image to match the others where many have gone before.

Testing instructions

Create a new account and site at /new and let yourself be swept away by the journey to the editor.

When the NUX modal appears, check that the images are of the same dimension and the jump affect is reduced.

Reported in #41966 (review)

@matticbot
Copy link
Contributor

@ramonjd ramonjd added [Goal] Full Site Editing [Goal] New Onboarding previously called Gutenboarding [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels May 26, 2020
@matticbot
Copy link
Contributor

Caution: This PR affects files in the FSE Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D43989-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing in the FSE Plugin for more info: PCYsg-ly5-p2

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@ramonjd
Copy link
Member Author

ramonjd commented May 26, 2020

The replacement image for preview.svg looks good and removes the height jump reported in #41966 (review)

When I'm testing however using yarn dev --sync things don't look right at all:

fse-nux

This is both on this PR and a clean master branch (and clean sandbox before sync) 🤷

Usually when I ask the question "Is it just me or...?" I get the answer: "Yes, it's just you."

Hoping to get that answer here as well :)

It should look like (created with a new site + account on Gutenboarding in prod)

Screen Shot 2020-05-28 at 7 08 06 am

@ramonjd ramonjd requested a review from joanrho May 26, 2020 22:29
Copy link
Member

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good to me after sandboxing D43989-code!

@ramonjd
Copy link
Member Author

ramonjd commented May 28, 2020

Thanks @andrewserong I totally forgot that the build log shows us where to test. TIR (Today I Remembered)

@ramonjd ramonjd merged commit d5cb75d into master May 28, 2020
@ramonjd ramonjd deleted the update/fse-nux-preview-image-size branch May 28, 2020 23:45
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 28, 2020
@ockham ockham mentioned this pull request May 29, 2020
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants