-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Try: Contextual frame bg color to avoid artifacting. #62223
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +25 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
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 fine with me and the Safari issue is still fixed with this solution
Thanks for the review. I'll let this one sit for a bit just to give others a chance to chime in. |
// Show a dark background in "frame" mode to avoid edge artifacts. | ||
// Show a gray background in all other contexts, such as zoom. | ||
background-color: $gray-900; | ||
.is-full-canvas & { |
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.
is-full-canvas is a site editor class so this style should probably not live here. (same for the other file).
Good feedback, thanks for testing. I changed the approach to leverage the fact that the class was site editor only. What do you think? |
1ba8e30
to
81205a3
Compare
width: 100%; | ||
height: 100%; | ||
display: block; | ||
background-color: $gray-300; |
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.
So this background is not needed at all, even on the post editor?
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.
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.
In my testing, these contexts inherit from the other class.
What do you say, good to land?
Edit: as always, happy to follow up.
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.
This approach looks good to me as long as the removal of the background color is ok for the post editor too.
Should this be backported for WP 6.6? If so, please add the |
I'd like it backported, because it's a great fix for visually dark themes in the site editor. So I'll add the label, thanks for the reminder. It's also an entirely safe bugfix. But if it's chosen to not be backported, that also okay. |
Let's mark it as a bug fix then, the enhancement label confused me a bit |
* Try: Contextual frame bg color to avoid artifacting. * Try different fix. Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: MaggieCabrera <onemaggie@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
* Try: Contextual frame bg color to avoid artifacting. * Try different fix. Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: MaggieCabrera <onemaggie@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
* Try: Contextual frame bg color to avoid artifacting. * Try different fix. Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: MaggieCabrera <onemaggie@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
This was cherry-picked to the wp/6.6 branch. |
What?
When you have a theme with a dark background, the scale-up animation of the frame in the site editor has a white halo artifacting itself due to rounding math in the scale-up process:
before.mp4
This is due to the light gray background that is applied to the frame background peeking out.
This PR solves that by contextually applying a dark background to the framed contexts:
after.mp4
What about when you're using a light theme, you might ask? The artifacting is still addressed in that case because the outer background is dark. So it's only an issue when it's a dark background, a light frame background, and then a dark content background inside.
Testing Instructions
Use a dark background, test the site editor, mouse over the frame to scale it up.
Note that previously the light gray background was applied specifically to address an issue with zoom out mode. Please do test that this still works.