feat: apply hover effects anywhere on the component #3796
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
|
|
Visual diff tests failed - pull request #3802 has been opened with the updated goldens. |
components/inputs/input-styles.js
Outdated
| border-width: 2px; | ||
| outline-style: none; | ||
| outline-width: 0; | ||
| padding: var(--d2l-input-padding-focus, calc(0.4rem - 1px) calc(0.75rem - 1px)); | ||
| } | ||
| :host(:hover) .d2l-input:not([aria-invalid="true"]), |
There was a problem hiding this comment.
Targeting :host(:hover) assumes that the consumer of these styles is a simple input component wrapper, not a component composed of multiple elements. I didn't dig into them but it does appear that app components are also consuming inputStyles directly. I think we need to assume they could be pulling those styles into a huge component composed of lots of different things, possibly even multiple inputs. Even if those cases are "ok" with this change, this does still make me feel a bit uneasy. I think it would be better if there was an element that we could target that would isolate this behaviour, like one of the wrapper divs. 🤔
There was a problem hiding this comment.
Agreed. These input styles are intended to be used standalone on a <input class="d2l-input"> and as Dave mentioned the "host" may be a much larger component composed of many different things.
I think if we want to do this it'll have to be something constrained to <d2l-input-text>, which is the thing that knows about the label. We may need to programatically add/remove a .d2l-input-hover CSS class when the host is hovered/blurred, and then add CSS here to handle it -- similar to how this already understands .d2l-input-focus.
|
🎉 This PR is included in version 2.136.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
ghost
added
the
Apply the input hover effects when hovering over anywhere on the component, not just the input or label.
Rally: US148080