[ui] Fix unnecessary middle truncation occurring in dialogs #26086
Conversation
|
Deploy preview for dagit-storybook ready! ✅ Preview Built with commit 971c853. |
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 971c853. |
| const width = container.getBoundingClientRect().width; | ||
|
|
||
| // https://developer.mozilla.org/en-US/docs/Web/API/CSS_Object_Model/Determining_the_dimensions_of_elements#how_much_room_does_it_use_up | ||
| const width = container.offsetWidth; |
There was a problem hiding this comment.
Not caused by your PR but can we wrap the usage of calculateMiddleTruncatedText in a requestAnimationFrame since accessing offsetWidth triggers a forced layout.
There was a problem hiding this comment.
Hmm it looks like this fn is called in two places - one in a useLayoutEffect and one in a resize observer. I think in the resizeObserver case the layout is fresh so it's best to read it immediately vs allowing to change and then requesting it again, but I'm not sure about the useLayoutEffect callsite.
It looks like using useLayoutEffect instead of useEffect is the preferred way to implement this without an empty frame where the text was null while we were waiting to determine it's size, but if we're ok with the empty frame + not forcing a render, i think we switch that to useEffect and add a window.requestAnimationFrame.
I don't think it'll be that bad to have the null text for a single frame, I'll switch it and we can see if it's noticeable 👍
There was a problem hiding this comment.
Delay of doing it in a requestAnimationFrame isn't noticeable as far as I can tell - switched!
bengotow
force-pushed
the
bengotow-2024-11/FE-684
branch
from
2bca56d
to
971c853
Compare
Summary & Motivation
https://linear.app/dagster-labs/issue/FE-684/targeted-assets-dialog-is-too-aggressive-with-middle-truncate
Thankfully this turned out to be a quick fix - I verified in the docs that
offsetWidthshouldn't have other implications beyond this:How I Tested These Changes
Tested manually in the "Upstream assets modal" with some long asset names and also via a new storybook + verification that no other MiddleTruncate storybooks were impacted.
Changelog
[ui] Fixed unnecessary middle truncation occurring in dialogs.