refactor(tooltip): avoid calc inside transform for IE support
#3739
Conversation
|
Deploy preview for the-carbon-components ready! Built with commit cf3a9fd https://deploy-preview-3739--the-carbon-components.netlify.com |
|
Deploy preview for carbon-elements ready! Built with commit cf3a9fd |
|
Deploy preview for carbon-components-react ready! Built with commit cf3a9fd https://deploy-preview-3739--carbon-components-react.netlify.com |
|
@asudoh any thoughts on the |
|
@emyarod Thank you for double-checking, either way is fine. I'd choose "all browsers" solution if it's straightforward, and choose IE-override solution otherwise. |
|
@emyarod Should we go ahead with this PR or do you still have some open questions...? Thanks! |
|
@asudoh I will add IE overrides so that we can continue to use |
emyarod
force-pushed
the
3671-tooltip-avoid-calc-inside-transform
branch
from
a2725c8
to
d618545
Compare
|
tooltip width is now hard coded for IE, so users will have to modify that to fit their tooltip content on IE |
|
@emyarod Thank you for the update - The latest code LGTM 👍, do you think this PR is ready to go? |
|
@asudoh yes if IE looks good for the reviewers as well |
|
@carbon-design-system/design does this look good? |
|
@emyarod Is this refactor supposed to not change the design/behavior? There are some changes to the tooltips from a quick look – is this PR supposed to change how alignments are done? If not calculated, how are the triangles going to line up to "start, center, end"? Eg. below screenshot is supposed to be end alignment, but is not correct:
|
|
@shixiedesign this is a refactor, so the behavior is identical to what we settled on previously. IIRC we decided the caret would always be centered and the alignment would only affect the tooltip body
the result of our discussion #2748 (comment) |
emyarod
force-pushed
the
3671-tooltip-avoid-calc-inside-transform
branch
2 times, most recently
from
912c9da
to
5f99102
Compare
|
@vincentsnagg got it, I was testing on Chromium Edge but I just added a feature query for Edge 12-15 and 16 |
emyarod
added
status: ready for review 👀
status: visual review 🔍
and removed
status: visual review 🔍
status: WIP 👩💻
labels
|
@shixiedesign Thank you for doing the design review! Does the latest comment from @emyarod make sense? Probably if something has to be changed from UX perspective we can enter a new issue to track it...? |
emyarod
force-pushed
the
3671-tooltip-avoid-calc-inside-transform
branch
from
bb57856
to
ee8aa15
Compare
Closes #3671
Related: #2781, #3115, #3151
This PR refactors tooltip positioning logic to avoid
calc()usage inside of CSStransform(https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/104773/). We now use a combination of TRBL positioning and CSS transforms rather than relying solely on CSS transforms.We use explicit
remvalues for TRBL positioning (dictated by the spec), and we use percentage values to further translate the position of the tooltip caret and body depending on the desired position and alignmentChangelog
New
displayChanged
transformto TRBL +transform)transformto TRBL +transform)Removed
initialandmax-contentvalues in IETesting / Reviewing
Ensure tooltips appear correct on all platforms and all existing use cases (including password input, icon-only button)
Regarding the point made by @tw15egan in the issue, we have a few options WRT
width: max-contentusage:we can either apply a workaround for all browsers or add IE specific rules (I guess this is progressive enhancement vs graceful degradation?)