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

Fixed style flickering caused by the border #24739

Merged
merged 4 commits into from
Aug 31, 2020

Conversation

aktasfatih
Copy link
Member

@aktasfatih aktasfatih commented Aug 23, 2020

Description

In block-preview, when a style is selected, the style gets a 2px wide border, which causes a small flickering effect. This PR adds a transparent 2px border so that the style doesn't flicker with the new is-active border.
Closes #24740

How has this been tested?

Tested it on ubuntu firefox.
1- Add an image
2- Click in between styles

Screenshots

Before:
before

After:
working

Types of changes

Added a new line of style for the border.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@youknowriad youknowriad added [Type] Bug An existing feature does not function as intended Needs Design Feedback Needs general design feedback. labels Aug 24, 2020
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Makes sense to me

@youknowriad youknowriad requested a review from jasmussen August 24, 2020 10:12
@jasmussen
Copy link
Contributor

Thanks for the ping Riad.

Is there any other way we can address this shift, than using a transparent border? I ask because in Windows high contrast mode, the transparent border will be made opaque effectively making it impossible to see which style is active or not.

@youknowriad
Copy link
Contributor

Maybe the active style could use an outline instead of borders.

@talldan
Copy link
Contributor

talldan commented Aug 25, 2020

This may also fix #24611, although that does seem to be happening without the style selection changing, possibly still worth testing though.

@aktasfatih
Copy link
Member Author

aktasfatih commented Aug 25, 2020

I realized this when I was looking at #24611 haha. However, I couldn't replicate that scrollbar issue even though I tried exactly the same resolution given in that issue.

@jasmussen, Yes you are right. It is impossible to see the selected one. I didn't know we had to think about high contrast. Thanks!
Screenshot_7

The table below shows how it looks like with the outline instead of the border. (Which is my last commit) What do you think?

Normal High Contrast
after2 after2highc

@jasmussen
Copy link
Contributor

Thank you for trying high contrast mode!

With the outline we're losing the border radius on the active item. Is there any way we could avoid that? Could we use a contextual padding on the preview so it doesn't rerender when unselected?

Thanks for your patience!

@talldan talldan added the [Feature] Theme Style Variations Related to style variations provided by block themes label Aug 28, 2020
@aktasfatih
Copy link
Member Author

Padding Margins
Screenshot_3 Screenshot_5)

That's smart. However, paddings created gaps. Margins seem to work without creating gaps between the component and the borders. How does it look? The image at the right is my last commit.

@jasmussen
Copy link
Contributor

That seems alright to me! Does the non-highcontrast mode still look good? Thanks!

@aktasfatih
Copy link
Member Author

Non-highcontrast is pretty much the same.
Screenshot_6

@jasmussen
Copy link
Contributor

Ship it!

@youknowriad
Copy link
Contributor

Let's get that in. Thanks for the fix @aktasfatih

@youknowriad youknowriad merged commit 1930a97 into WordPress:master Aug 31, 2020
@github-actions github-actions bot added this to the Gutenberg 8.9 milestone Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Theme Style Variations Related to style variations provided by block themes Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block styles are flickering when changed
4 participants