-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
chore(cell-actions): use popper strategy fixed instead of portalling #97593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(cell-actions): use popper strategy fixed instead of portalling #97593
Conversation
nikkikapadia
left a comment
There was a problem hiding this 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' |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
…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.
…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.
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-v2is enabled.