Skip to content

Commit

Permalink
[ui] Fix unnecessary middle truncation occurring in dialogs (#26086)
Browse files Browse the repository at this point in the history
## 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 `offsetWidth` shouldn't have other implications beyond this:

<img width="754" alt="image"
src="https://github.com/user-attachments/assets/000c76f6-cfcb-4a47-a83b-2436b18acc8b">


## 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.

<img width="371" alt="image"
src="https://github.com/user-attachments/assets/35fa3889-9609-46a4-9ca3-303464b70494">


## Changelog

[ui] Fixed unnecessary middle truncation occurring in dialogs.

Co-authored-by: bengotow <[email protected]>
  • Loading branch information
bengotow and bengotow authored Nov 24, 2024
1 parent 8794192 commit d298e05
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ export const MiddleTruncate = ({text, showTitle = true}: Props) => {

// Use a layout effect to trigger the process of calculating the truncated text, for the
// initial render.
React.useLayoutEffect(() => {
calculateTargetStyle();
React.useEffect(() => {
window.requestAnimationFrame(calculateTargetStyle);
}, [calculateTargetStyle]);

// If the container has just been resized, recalculate.
Expand Down Expand Up @@ -87,7 +87,9 @@ const Container = styled.div`
*/
const calculateMiddleTruncatedText = (container: HTMLDivElement, text: string) => {
const font = getComputedStyle(container).font;
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;

const body = document.body;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,36 @@ export const Simple = () => {
);
};

export const TransformedContainerUsage = () => {
return (
<Box>
<em style={{display: 'block', marginBottom: 10}}>
Note: Only the first item should appear truncated. This use case is based on our usage of
MiddleTruncate in modals that animate in.
</em>
{[
'asset_that_supports_partition_ranges',
'asset_downstream',
'asset_weekly_root',
'asset_weekly',
].map((text) => (
<Box
key={text}
style={{maxWidth: 200, transform: 'scale(0.8)'}}
flex={{direction: 'row', gap: 8}}
>
<Box>
<Icon name="asset_non_sda" />
</Box>
<a style={{overflow: 'hidden'}} href="#/">
<MiddleTruncate text={text} />
</a>
</Box>
))}
</Box>
);
};

export const FlexboxContainerUsage = () => {
return (
<Box>
Expand Down Expand Up @@ -85,6 +115,7 @@ export const FlexboxContainerUsage = () => {
</Box>
);
};

export const TagUsage = () => {
return (
<Tag icon="job">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const AssetKeysDialog = (props: Props) => {
<Dialog
isOpen={isOpen}
onClose={() => setIsOpen(false)}
style={{width: '750px', maxWidth: '80vw', minWidth: '500px', transform: 'scale(1)'}}
style={{width: '750px', maxWidth: '80vw', minWidth: '500px'}}
canOutsideClickClose
canEscapeKeyClose
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,12 @@ export const BackfillPreviewModal = ({
);
}, [data]);

// BG Note: The transform: scale(1) below fixes a bug with MiddleTruncate where the text size
// is measured while the dialog is animating open and the scale is < 1, causing it to think
// it needs to truncate. A more general fix for this seems like it'll require a lot of testing.

return (
<Dialog
title="Backfill preview"
isOpen={isOpen}
onClose={() => setOpen(false)}
style={{width: '90vw', maxWidth: 1100, transform: 'scale(1)'}}
style={{width: '90vw', maxWidth: 1100}}
>
<Container
ref={parentRef}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export const TickDetailsDialog = ({
<Dialog
isOpen={isOpen}
onClose={onClose}
style={{width: '80vw', maxWidth: '1200px', minWidth: '600px', transform: 'scale(1)'}}
style={{width: '80vw', maxWidth: '1200px', minWidth: '600px'}}
>
<TickDetailsDialogImpl
tickId={tickId}
Expand Down

2 comments on commit d298e05

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for dagit-storybook ready!

✅ Preview
https://dagit-storybook-htadh7ohk-elementl.vercel.app

Built with commit d298e05.
This pull request is being automatically deployed with vercel-action

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-5g7m23aou-elementl.vercel.app

Built with commit d298e05.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.