Skip to content

Conversation

@JonasBa
Copy link
Member

@JonasBa JonasBa commented Jan 22, 2026

Applies Container codemod to Components that do not have any codeowners

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jan 22, 2026
Comment on lines 209 to 219
// Evenly distributing the cards into columns.
const columnSize = Math.ceil(cards.length / columnCount);
for (let i = 0; i < cards.length; i += columnSize) {
columns.push(<CardColumn key={i}>{cards.slice(i, i + columnSize)}</CardColumn>);
columns.push(
<Container column="span 1" key={i}>
{cards.slice(i, i + columnSize)}
</Container>
);
}

return (
Copy link

Choose a reason for hiding this comment

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

Bug: A local function named Container conflicts with an imported component of the same name. This causes the function to incorrectly render the imported component instead of its intended content.
Severity: HIGH

Suggested Fix

Rename the local Container function to avoid the naming collision with the imported component from @sentry/scraps/layout. For example, renaming it to KeyValueDataContainer and updating its export and usage within the file will resolve the shadowing issue.

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/keyValueData/index.tsx#L209-L219

Potential issue: In `keyValueData/index.tsx`, a local function `Container` is defined,
but there is also an `import {Container}` from `@sentry/scraps/layout` at the module
level. Inside the local `Container` function, it attempts to render a `<Container>`
component. Due to JavaScript's scoping rules, this resolves to the imported `Container`
component from `@sentry/scraps/layout`, not the local function itself. This leads to
incorrect rendering behavior, as the component effectively calls the imported layout
component with props it may not be designed for, breaking the intended layout logic
wherever `KeyValueData.Container` is used.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 6 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

})``;
export function ScoreWrapper(props: ContainerProps<'div'>) {
return <Container {...props} />;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

ScoreWrapper missing flex layout properties

High Severity

The ScoreWrapper component was converted from a Flex component with align="baseline" and maxWidth="100%" to a plain Container without these properties. The child Score component uses flex-shrink: 1, which requires a flex container parent to work correctly. This breaks the intended flex layout and alignment.

Fix in Cursor Fix in Web

) : (
// Placeholder to maintain layout when no version is selected
<PlaceholderLink aria-hidden="true" />
<Container as="span" minHeight="1.2em" aria-hidden="true" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing display property on span with minHeight

Medium Severity

The placeholder element was changed from a styled span with display: inline-block and min-height: 1.2em to <Container as="span" minHeight="1.2em"> without the display: inline-block. The minHeight property has no effect on inline elements (the default for span), so the layout placeholder won't maintain the intended minimum height.

Fix in Cursor Fix in Web

details: (
<Container as="span" width="20rem">
{r.desc}
</Container>
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing display property on span with width

Medium Severity

The role description container was changed from a styled span with display: inline-block and width: 20rem to <Container as="span" width="20rem"> without the display: inline-block. The width property has no effect on inline elements (the default for span), so the role descriptions won't have the intended fixed width constraint.

Fix in Cursor Fix in Web

</InputGroup.TrailingItems>
</InputGroup>
</SearchWrapper>
</Container>
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing container query responsive display property

Medium Severity

The search input container lost its container query that hides it at small widths. The original SearchWrapper had @container (max-width: 500px) { display: none; } to responsively hide the search at narrow container widths, but this was removed when converting to Container. The search input will now always be visible regardless of container width.

Fix in Cursor Fix in Web

<StyledPlatformIcon platform={projectPlatforms[0]!} size={18} />
<BorderOverlay />
</IconContainer>
</Container>
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing display grid on icon container

Medium Severity

The icon container lost display: grid when converted to Container. The original IconContainer used grid layout to position the platform icons, but this property wasn't carried over. The child PlatformIconWrapper components use absolute positioning and expect a grid context, so the icon layout will be broken.

Additional Locations (1)

Fix in Cursor Fix in Web

<Container margin="lg 0" data-separator>
<Separator />
</SeparatorWrapper>
</Container>
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing default display none on separator

Medium Severity

The separator container lost its default display: none property when converted to Container. The original SeparatorWrapper was hidden by default and conditionally shown via a parent selector. Without the default hidden state, separators will always be visible instead of only appearing between navigation sections.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants