[DataGrid] Align grid cell tooltip with the column header #14484
Conversation
arminmeh
added
accessibility
|
Deploy preview: https://deploy-preview-14484--material-ui-x.netlify.app/ |
arminmeh
force-pushed
the
grid-cell-tooltip
branch
from
1381ec3
to
1dcf036
Compare
arminmeh
force-pushed
the
grid-cell-tooltip
branch
2 times, most recently
from
ce452ad
to
5e90ef0
Compare
| @@ -30,6 +30,12 @@ const GridColumnHeaderTitleRoot = styled('div', { | |||
| whiteSpace: 'nowrap', | |||
| fontWeight: 'var(--unstable_DataGrid-headWeight)', | |||
| lineHeight: 'normal', | |||
| // To prevent Safari adding its own tooltip for truncated text | |||
There was a problem hiding this comment.
This issue is still present on the GridCell, but I couldn't apply the same because it would impact performance
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
arminmeh
force-pushed
the
grid-cell-tooltip
branch
from
0f07bbe
to
c380185
Compare
github-actions
bot
added
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
arminmeh
force-pushed
the
grid-cell-tooltip
branch
from
c380185
to
e067672
Compare
github-actions
bot
added
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
arminmeh
force-pushed
the
grid-cell-tooltip
branch
from
e067672
to
236b89b
Compare
github-actions
bot
removed
the
PR: out-of-date
| const tooltipProps = slotProps?.tooltip || { | ||
| slotProps: { | ||
| popper: { | ||
| sx: { | ||
| [`&.${tooltipClasses.popper}[data-popper-placement*="bottom"] .${tooltipClasses.tooltip}`]: | ||
| { | ||
| marginTop: '-5px', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Looks like a static object, can be extracted.
| @@ -7,8 +7,10 @@ import { | |||
| unstable_ownerDocument as ownerDocument, | |||
| unstable_capitalize as capitalize, | |||
| } from '@mui/utils'; | |||
| import { tooltipClasses, TooltipProps } from '@mui/material/Tooltip'; | |||
There was a problem hiding this comment.
Do you see any way to do this without adding material imports here? I need to get rid of these for the design-system agnostic OKR.
There was a problem hiding this comment.
This PR is not explicitly requested by anyone. I just saw that we are using different approach for the header row cell and content row cell tooltips.
If it creates too much trouble, the PR can be discarded.
What will be the replacement for the current header cell tooltip?
| @@ -76,6 +78,7 @@ export type GridCellProps = { | |||
| onKeyDown?: React.KeyboardEventHandler<HTMLDivElement>; | |||
| onDragEnter?: React.DragEventHandler<HTMLDivElement>; | |||
| onDragOver?: React.DragEventHandler<HTMLDivElement>; | |||
| slotProps?: { tooltip?: Partial<TooltipProps> }; | |||
There was a problem hiding this comment.
Avoiding exposing material types on our public API is particularly important to achieve the design-system agnostic OKR :| If this is unavoidable, any way to create a minimal subset of the type to avoid exposing everything?
While working on #9954 I have noticed that sometimes the cell values are read twice as well.
This occurs because of the
titleprop that was added in #7671Accessibility tools were already showing warnings for this approach.
Since column header component is using tooltip to show the whole content, I have added the same to the cells and removed the
titleprop.This introduced a small UX issue, since the cell text content is added directly to the cell container to reduce the number of nodes. Tooltip shows up way below the cell, because it has a default margin relative to the target element.
I have added a default
slotPropvalue for theTooltipcomponent, to render it just a bit over the bottom borderWith default margins
With mt = -5px
To allow changes, I repeated this pattern (but with default)
Issue that should bring this topic even further #7323