Skip to content
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] Wrap column menu button with a tooltip #7890

Merged
merged 1 commit into from Apr 5, 2023

Conversation

cherniavskii
Copy link
Member

Closes #7877

@cherniavskii cherniavskii added component: data grid This is the name of the generic UI component, not the React module! plan: Premium Impact at least one Premium user enhancement This is not a bug, nor a new feature needs cherry-pick The PR should be cherry-picked to master after merge labels Feb 10, 2023
@mui-bot
Copy link

mui-bot commented Feb 10, 2023

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-7890--material-ui-x.netlify.app/

Updated pages

No updates.

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 620.7 1,013.5 712 839.64 150.807
Sort 100k rows ms 724.8 1,401.7 1,401.7 1,088.28 234.254
Select 100k rows ms 254.9 400.2 299.9 307.5 49.359
Deselect 100k rows ms 184.4 391.1 255.8 264.18 72.512

Generated by 🚫 dangerJS against 81ffbb9

aria-controls={columnMenuId}
id={columnMenuButtonId}
{...rootProps.componentsProps?.baseIconButton}
enterDelay={1000}
Copy link
Member

Choose a reason for hiding this comment

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

Is this delay a little bit above the user's expectation? As it's supposed to work in place of the default HTML tooltip which I guess is 0.3-0.5 seconds, do you think the delay should be set to a smaller number?

The MUI Tooltip also has a default delay of 0.1s and material design shows a minor delay in the image.

The same (1s) is the case for the filter button tooltip though (maybe there was a reason behind that which I am not aware of 🤔 )

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I used the same delay that was being used for the filter button tooltip.
I'm not sure why this exact value was used, maybe to simulate the behavior of aria-label that is not shown immediately, but after a delay?

Copy link
Member

Choose a reason for hiding this comment

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

Probably somewhere around the default delay for HTML title attribute (0.3-0.5 seconds) would be closer to a normal user's expectation.
Not a big deal though. Being consistent with the filter button tooltip also seems fine to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably somewhere around the default delay for HTML title attribute (0.3-0.5 seconds) would be closer to a normal user's expectation.

Maybe it depends on the browser, but in Chromium, it seems to be more than 0.5s:

Screen.Recording.2023-04-05.at.19.06.43.mov

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 18, 2023
@github-actions

This comment was marked as outdated.

@cherniavskii cherniavskii marked this pull request as ready for review April 4, 2023 19:15
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 4, 2023
@cherniavskii cherniavskii merged commit 06dcac2 into mui:master Apr 5, 2023
16 checks passed
@cherniavskii cherniavskii deleted the column-menu-button-tooltip branch April 5, 2023 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature needs cherry-pick The PR should be cherry-picked to master after merge plan: Premium Impact at least one Premium user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid premium] Customizing ColumnMenuButton and Tooltip
3 participants