Fix color picker saturation overlay offset

[Addison-Stavlo]

Description

Fixes issue #19610 .

• Forces a scroll on mounting of the saturation component.
• Updates the translate offset for the picker to appropriately match its size.

Previously, when the saturation component loaded with the picker on the right side it would cause the scrollLeft value to be several pixels, causing the issue noted in #19610 .

Before this PR

In absence of finding a CSS rule to fix this behavior, forcing a scroll(0,0) in the parent element on mounting of the component will fix the issue.

After scroll on mount

But as you may notice, the pointer is now almost all the way off the screen. It turns out the transform: translate(x,y) property in the CSS was set for a picker of 8px width/height. Since the picker is now 14px wide/tall it was necessary to update this translate value as well. (Update, set the translate to use % based values so this will be less of an issue in the future)

After this PR

How has this been tested?

Tested on local docker environment.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

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

[jeryj]

I so wish we could track down whatever the weird CSS issue is to fix this 😭 But thank you so much for looking into this and putting in a fix! This seems like a low-risk fix that does address it 👍

[epiqueras]

This is an issue with how the browser renders button.

Changing it to a div with aria labels fixes this.

[jeryj]

Nice catch, @epiqueras! Semantically, I don't think this should end up being a button anyways. It doesn't have any Enter/Spacebar function or submit anything.

Maybe there's a better role to use than button (or leave it off). I did a quick skim of the aria roles, and nothing stands out to me as entirely appropriate.

[Addison-Stavlo]

Changing it to a div with aria labels fixes this.

It does at first, but as soon as we make it tab-able again the problem comes back. I assume we want to keep it tab-able for a11y concerns.

I tried changing it to a <div /> and added role="button" (or similarly 'slider') and tabIndex="0". It seems to be the tab a11y that causes this issue (whether its a button or div)? So I left it as the <Button /> component for now since they both seem to have the same issue.

(also rebased)

[jeryj]

Yeah, it would definitely need to remain focusable. I'd never heard of adding tabindex being an issue for layout! Bummer 😞

I think we can drop the role="button" though. The selector really isn't operating as a traditional button.

[Addison-Stavlo]

I think we can drop the role="button" though. The selector really isn't operating as a traditional button.

Right, IF we went with the <div /> it does need a role, similarly we could call it "slider" (im struggling to find better options than button or slider). BUT since the <div /> fix doesn't solve the original issue and requires other style changes to get the same visual appearance it probably doesn't make sense to go that route anyways?

[epiqueras]

Removing this:

.components-color-picker__saturation-color {
	overflow: hidden;
}

fixes it.

[Addison-Stavlo]

Removing this:
fixes it.

It fooled you too (if you are seeing what I am?). We tried that earlier too 😢 . What happens is it just replaced the problematic region on the right with a white bar instead of a colored one.

[jasmussen]

Apologies for missing the ping. I'm looking into this now, and I'm almost certain there's a CSS only fix for this. I'm not really able to reproduce the issue as it was back in january, but I'll keep looking.

[jasmussen]

I created #23272 as a CSS only alternative to this one. Can you test it and let me know if it fixes the issue for you?

[Addison-Stavlo]

Thanks @jasmussen ! It is relieving to see that there is a CSS-only fix for this. We were aiming for that but never were able to find a working solution. That other PR #23272 solves the issue and I will close this in favor of that.

I'm not really able to reproduce the issue as it was back in january

I wasn't able to at first, then I found that the issue changed slightly. Instead of being triggered by the far right edge of the container, it moved to being triggered by the bottom edge. However, the #23272 fixes this as noted. 🎉