Skip to content

Conversation

@lzhao-sentry
Copy link
Contributor

@lzhao-sentry lzhao-sentry commented Aug 11, 2025

Changes

Instead of portals, use the fixed strategy. Main reason is that portalling the dropdown to document body does not work with tables in modals due to stacking issues (e.g., planning to add cell actions to viewer table modal in dashboards)

There should be no UI change to the dropdown. This change only applies when discover-cell-actions-v2 is enabled.

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Aug 11, 2025
@lzhao-sentry lzhao-sentry marked this pull request as ready for review August 11, 2025 20:15
@lzhao-sentry lzhao-sentry requested a review from a team as a code owner August 11, 2025 20:15
Copy link
Member

@nikkikapadia nikkikapadia left a comment

Choose a reason for hiding this comment

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

just a prop naming comment

| 'preventOverflowOptions'
| 'flipOptions'
| 'shouldApplyMinWidth'
| 'strategy'
Copy link
Member

Choose a reason for hiding this comment

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

can we name this something else so we know exactly what it's for? maybe something like dropdownPortalStrategy or whatever (maybe don't use that name it doesn't make sense but you catch my drift)

Copy link
Contributor Author

@lzhao-sentry lzhao-sentry Aug 14, 2025

Choose a reason for hiding this comment

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

It's built into the popperJS library. I think useOverlayProps has a comment explaining it, but I can add a better comment in the dropdown menu file as well

Copy link
Member

Choose a reason for hiding this comment

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

ya if the name can't be changed then let's add a more detailed comment about it

@lzhao-sentry lzhao-sentry merged commit c3db7c1 into master Aug 15, 2025
45 checks passed
@lzhao-sentry lzhao-sentry deleted the lzhao/chore/use-fixed-strategy-over-portals branch August 15, 2025 16:26
priscilawebdev pushed a commit that referenced this pull request Aug 25, 2025
…97593)

### Changes

Instead of portals, use the fixed strategy. Main reason is that
portalling the dropdown to document body does not work with tables in
modals due to stacking issues (e.g., planning to add cell actions to
viewer table modal in dashboards)

There should be no UI change to the dropdown. This change only applies
when `discover-cell-actions-v2` is enabled.
andrewshie-sentry pushed a commit that referenced this pull request Aug 26, 2025
…97593)

### Changes

Instead of portals, use the fixed strategy. Main reason is that
portalling the dropdown to document body does not work with tables in
modals due to stacking issues (e.g., planning to add cell actions to
viewer table modal in dashboards)

There should be no UI change to the dropdown. This change only applies
when `discover-cell-actions-v2` is enabled.
@github-actions github-actions bot locked and limited conversation to collaborators Aug 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants