fix: Align hover ripple for checkbox and radio components in IE11

[lychyi]

Summary

Fixes #650

Explanation

I'll do my best to explain how this fix came to be for both Radio and Checkbox because they are implemented similarly.

Align browsers by explicit positioning (i.e. change top: auto to top: 0px)

The first thing was to fix ripple positioning in IE11. This means that we have to make sure that the ripple is placed intentionally by setting top: 0px instead of the default top:auto which is browser controlled.

Unfortunately, that introduces offset errors due to border-width changes

Hang on, not that simple though. The width of the border for the parent element (RadioBackground, CheckboxBackground) will render with offset errors on all browsers when this happens.

Why? This is because absolutely positioned element coordinates are based on the parent's content not the content + border width (border-box doesn't help this). So to align correctly you have to account for the border's width, i.e. top: -1px. If the border changes to 2px (which it does), this becomes problematic since you also have to adjust the ripple's positioning. They're tightly coupled.

I tried, and as I adjusted for the changing border-width the visual state tables would look correct, I got offset errors while transitioning between states with different border sizes! This is not good and really hard to reason about and impossible to catch with our tests today.

Offset errors look something like this:

Solution: Move it so it wouldn't be influenced by changes in parent's border-width

The solution was to move the ripple to a place where it wouldn't be influenced by changes in border-width from the background components. Moving it to be a sibling of the background component makes positioning a bit easier to reason about since changes in one doesn't affect the other.

Still needs to be connected to the input though

But one requirement is that the ripple animates via the input element's state changes (hover, active, disabled, etc.) so instead of a keeping it as a pseudo element (originally ::after), it was changed to a concrete element (span) so it could be targeted in a CSS sibling selector (~ span:first-of-type).

An alternative solution

An alternative was to transition everything to box-shadows because that doesn't influence positioning. You can't change individual components of it like you can a border (border-color, border-width, etc.) so that change worked but was also more code and nasty-looking ternaries when we wanted to have new box-shadows based on props.

Checklist

• branch has been rebased on the latest master commit
• yarn test passes (No tests were added because this regression already exists as a baseline 😓 )
• Fix for Radio
• Fix for Checkbox
• Inconsistencies with the Figma spec (could be a different issue/PR)

[cypress]

---

Test summary

317 • 0 • 0 • 0

---

Run details

Project	canvas-kit
Status	Passed
Commit	f7ad192 ℹ️
Started	May 21, 2020 8:24 PM
Ended	May 21, 2020 8:27 PM
Duration	03:18 💡
OS	Linux Ubuntu Linux - 18.04
Browser	Electron 80

View run in Cypress Dashboard ➡️

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[lychyi]

Do not merge, bugs still be out there 🐛

[NicholasBoll]

I think the changes look good (meaning the Focus + Hover shows the focus on top of the hover vs the reverse).

[lychyi]

BTW, Chromatic is not catching changes with Checkbox even though I'm clearly fixing it (changing it) here. It's not catching all the changes with Radio either. The diffs don't catch the hover ripple moving places which is a little troubling. I've opened up an issue with the Chromatic team, for now we'll just have to use our eyes to make sure the changes are good to go. 👀

[jamesfan]

Nice explanation -- you're right, reads like a novel 📖 .