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

Hover over theme patterns displays the default cursor instead of a pointer (hand) and there's no outline #128

Closed
ndiego opened this issue Jun 17, 2024 · 7 comments
Labels
[Component] Theme Templates, patterns, CSS [Type] Bug Something isn't working

Comments

@ndiego
Copy link
Member

ndiego commented Jun 17, 2024

Basically, the hover states for patterns on single theme pages are not the same as the hover states for style variations, and I think they should be the same. Patterns are missing the highlighted border and the pointer cursor style. Unfortunately the screen recording tool I'm using is not picking up the different cursor styles, but you can test by looking at the Twenty Twenty-Four page.

We also might want some transition applied to the animation, but I leave that to @WordPress/meta-design

hover-states.mp4
@ndiego ndiego added [Component] Theme Templates, patterns, CSS [Type] Bug Something isn't working labels Jun 17, 2024
@ndiego
Copy link
Member Author

ndiego commented Jun 17, 2024

Related to #126

@ryelle
Copy link
Collaborator

ryelle commented Jun 17, 2024

We also might want some transition applied to the animation, but I leave that to @WordPress/meta-design

Ah, I was going to do that.

Patterns are missing the highlighted border and the pointer cursor style.

Good catch on the cursor 👍🏻 The styles for this were only designed for the previewer, so I kind of … backported them to something that makes sense on this page. I agree the border change should be the same though, and since it was just updated in plugins, I'll go ahead and have the hover darken border color on patterns too.

@StevenDufresne
Copy link
Contributor

StevenDufresne commented Jun 18, 2024

The pattern border also appears to be thicker than the screenshot and style variations border.
Screenshot 2024-06-18 at 10 23 18 AM

@jasmussen
Copy link

Good issue. A few things here, let's add a transition on both. Something a la transition: all 0.2s ease; does this:

ease

That's a choppy GIF but hopefully the animation comes across. For reduced motion we have this mixin in the block editor:

@mixin reduce-motion($property: "") {

	@if $property == "transition" {
		@media (prefers-reduced-motion: reduce) {
			transition-duration: 0s;
			transition-delay: 0s;
		}
	} @else if $property == "animation" {
		@media (prefers-reduced-motion: reduce) {
			animation-duration: 1ms;
			animation-delay: 0s;
		}
	} @else {
		@media (prefers-reduced-motion: reduce) {
			transition-duration: 0s;
			transition-delay: 0s;
			animation-duration: 1ms;
			animation-delay: 0s;
		}
	}
}

Steve is right about the border width, there's also something off about the radius:

Screenshot 2024-06-18 at 08 41 26

It's because there's an abs-positioned element on top of a pattern preview, and the outer radius is 4px, the inner radius is 3, but it doesn't fully add up. I wonder if the border could be set on .wporg-theme-listbox .wp-block-wporg-screenshot-preview instead? It should be 4px radius, 1px width, 1.5px only on focus. Can have the same easing.

@ryelle
Copy link
Collaborator

ryelle commented Jun 18, 2024

Moving the issue from #139 to a comment here, it doesn't need to be a separate issue.

This is in nit-picky territory, and it's probably related to #128 (comment) and how hover/focus styles are drawn on thumbnail previews.

But in case a refactor allows us to improve things, there are small edge artifacts on thumbnails. It's not immediately visible:

Image

But if you zoom in, you can see the white artifacts:

Image

This is likely from the focus style being stacked on top of a white border, using a pseudo element, rather than being drawn on the parent instead.

@ryelle ryelle closed this as completed in 0d2281b Jun 18, 2024
ryelle added a commit that referenced this issue Jun 18, 2024
@ryelle
Copy link
Collaborator

ryelle commented Jun 18, 2024

Fixed:

  • Added transition to the image scale
  • Added transition to the focus/selected states
  • Fixed the border radius on the pseudo-element
  • Updated the cursor to pointer for hover states
  • Fixed border size on unselected patterns to match 1px on style variations, and darken on hover

The white artifact is due to the white background, which is necessary for the patterns, otherwise they look like this:

Screenshot 2024-06-18 at 4 16 27 PM

And it needs to be a pseudo-element so that I can nest "borders" & have an inset border. If it's applied to the parent, it's hidden behind the image (plus the change in border width would impact the container size, so everything would shift). I know it's strange, but I did do that for a reason.

Let me know if I missed anything.

@jasmussen
Copy link

And it needs to be a pseudo-element so that I can nest "borders" & have an inset border. If it's applied to the parent, it's hidden behind the image (plus the change in border width would impact the container size, so everything would shift). I know it's strange, but I did do that for a reason.

I'll take your word for it.

In some other contexts, we've used inset shadows to accomplish the same. If that gets paired with something like a outline: 2px solid transparent;, we account for Windows high contrast mode as well, which removes shadows, and forcibly changes any opacities to full-opacity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Theme Templates, patterns, CSS [Type] Bug Something isn't working
4 participants