-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[DataGrid] Fix column menu position when closing #3289
Conversation
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
@@ -150,12 +151,22 @@ function GridColumnHeaderItem(props: GridColumnHeaderItemProps) { | |||
ariaSort = sortDirection === 'asc' ? 'ascending' : 'descending'; | |||
} | |||
|
|||
React.useEffect(() => { | |||
if (!showColumnMenuIcon) { | |||
setShowColumnMenuIcon(columnMenuOpen); |
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.
To only trigger this effect when the columnMenuOpen
prop changes, should we remove this !showColumnMenuIcon
Updating from true
to true
is not a problem no ?
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.
The purpose of this effect is: showColumnMenuIcon
should always reflect the value of columnMenuOpen
UNLESS showColumnMenuIcon=true
.
It could be rewrote as setShowColumnMenuIcon(value => !value ? columnMenuOpen : value);
. By the time this effect run, columnMenuOpen
will already be false.
Updating from true to true is not a problem no ?
No, React won't rerender the component if the value didn't change.
Preview: https://deploy-preview-3289--material-ui-x.netlify.app/components/data-grid/#commercial-version
Related to #1712 (comment)
Before:
After: