ref(container) add ellipsis support

[JonasBa]

Adds container ellipsis support for Container. This is an alternative pattern to using

<Container width={...}>
  <Text ellipsis>Overflowing Text...</Text>
</Container>


<Container ellipsis width={...}>
  Overflowing Text...
</Container>

[cursor]

Responsive ellipsis values not correctly mapped to CSS

Medium Severity

The ellipsis prop is typed as Responsive<boolean>, allowing responsive objects like {base: true, md: false}. However, the implementation uses simple truthiness checks (p.ellipsis ? 'ellipsis' : undefined) which treat any object as truthy. When a responsive object is passed, it results in 'ellipsis' being applied unconditionally instead of mapping each breakpoint's boolean value to the corresponding CSS value. The responsive behavior is lost.

 

[JonasBa]

Not entirely sold on the approach... there is less utility as I have imagined initially

[sentry]

Bug: The text-overflow and white-space properties incorrectly use a ternary on the ellipsis prop. This bypasses responsive handling when ellipsis is an object, applying styles globally.
_{Severity: MEDIUM}

🔍 Detailed Analysis

When the ellipsis prop is a responsive object (e.g., {xs: true, md: false}), the ternary expressions p.ellipsis ? 'ellipsis' : undefined and p.ellipsis ? 'nowrap' : undefined evaluate the object as truthy. This causes them to immediately return the string values 'ellipsis' or 'nowrap', bypassing the responsive styling function rc(). Consequently, text-overflow and white-space are applied globally across all breakpoints, contrary to the intended responsive behavior which is correctly implemented for the overflow property. This creates an inconsistent and buggy API for a newly introduced feature.

💡 Suggested Fix

Refactor the text-overflow and white-space style definitions to use the rc() utility function directly with a resolver, similar to how the overflow property is handled. This will ensure that when p.ellipsis is a responsive object, the styles are generated with the correct media queries. Avoid using a simple truthiness check on the p.ellipsis prop.

🤖 Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/components/core/layout/container.tsx#L303-L309

Potential issue: When the `ellipsis` prop is a responsive object (e.g., `{xs: true, md:
false}`), the ternary expressions `p.ellipsis ? 'ellipsis' : undefined` and `p.ellipsis
? 'nowrap' : undefined` evaluate the object as truthy. This causes them to immediately
return the string values `'ellipsis'` or `'nowrap'`, bypassing the responsive styling
function `rc()`. Consequently, `text-overflow` and `white-space` are applied globally
across all breakpoints, contrary to the intended responsive behavior which is correctly
implemented for the `overflow` property. This creates an inconsistent and buggy API for
a newly introduced feature.

_{Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8369330}

[TkDodo]

On Text, we have this named wrap:

sentry/static/app/components/core/text/text.tsx

Line 139 in 776966b

white-space: ${p => (p.wrap ? p.wrap : p.ellipsis ? 'nowrap' : undefined)};

The more I think about it, the more I would like us to stay as close as possible to html attributes. For example, <Text bold> isn’t better than setting font-weight and <Text wrap=“nowrap”> isn’t better than <Text whiteSpace=“nowrap”>, especially if it works differently on Container and we also have a textWrap prop on Text.

I sometimes find myself looking at the implementation of our core components just to know which props exist because I can’t find what I’m looking for in auto complete. That’s partially because auto complete lists all html props so it’s not helpful, but also because I type fontWeight or textDecoration and don’t find anything because our props are named differently ...

[JonasBa]

That's fair feedback. I think we can pull usage and see if that sticks, or if it's not been a good addition.

[JonasBa]

Closing this because I'm not entirely sold on the solution