[DataGrid] Column management design updates#16630
Conversation
kenanyusuf
added
scope: data grid
|
Deploy preview: https://deploy-preview-16630--material-ui-x.netlify.app/ |
| width: '100%', | ||
| height: '4px', | ||
| animation: `${reveal} linear both`, | ||
| animationTimeline: '--scroll-timeline', |
There was a problem hiding this comment.
Uses animation-timeline to show scroll shadows.
The support for animation-timeline isn't the best, but these scroll shadows can be a progressive enhancement as they are only a minor UX improvement. Not worth implementing with JS.
| sx={fullWidth ? { width: '100%', margin: 0 } : undefined} | ||
| sx={(theme) => ({ | ||
| gap: 0.5, | ||
| margin: 0, | ||
| width: fullWidth ? '100%' : undefined, | ||
| [`& .${formControlLabelClasses.label}`]: { | ||
| fontSize: theme.typography.pxToRem(14), | ||
| }, | ||
| })} |
There was a problem hiding this comment.
I tried avoiding having an sx prop on the checkbox rendered in the grid cells because that property is very slow (at least until I have time to finish this, after that it will merely be slow), and it degrades the scrolling UX.
There was a problem hiding this comment.
Good to know—I guess this change won't impact scrolling performance since it only adds the sx prop for checkboxes with labels?
There was a problem hiding this comment.
Ah right, yes it should not affect scrolling performance in that case.
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
| } | ||
| tabIndex={-1} | ||
| onClick={handleSearchReset} | ||
| edge="end" |
There was a problem hiding this comment.
Could we make this spacing consistent too?
There was a problem hiding this comment.
Good spot. It looks like having an IconButton with size="small as an input adornment is problematic as it doesn't get positioned correctly. Using a regular sized icon button results in this:
The spacing on the end adornment is closer to the start adornment's spacing, but still not perfect:
There was a problem hiding this comment.
The only issue I see with setting the IconButton size to regular is that on hover it looks like this:
Could we work around the positioning of a small IconButton instead? 🤔
There was a problem hiding this comment.
Could we work around the positioning of a small IconButton instead?
Yeah, this is a better option, we can target small icon buttons specifically within end adornments to fix this 28ac9c6. Also fixes it in the quick filter 🎉
| onClick={() => toggleAllColumns(!allHideableColumnsVisible)} | ||
| name={apiRef.current.getLocaleText('columnsManagementShowHideAllText')} | ||
| label={apiRef.current.getLocaleText('columnsManagementShowHideAllText')} | ||
| density="compact" |
There was a problem hiding this comment.
Does it make sense to set it (and other styling like spacing etc) according to rootProps.density?
There was a problem hiding this comment.
Potentially, yes. I am thinking about how the other UI elements should respond to the density setting but it's going to require a bit of experimentation. Will create a follow up issue to review this across the board.
github-actions
bot
removed
the
PR: out-of-date
noraleonte
left a comment
noraleonte
left a comment
There was a problem hiding this comment.
Awesome work 🚀 Looks great 💙
| } | ||
| tabIndex={-1} | ||
| onClick={handleSearchReset} | ||
| edge="end" |
There was a problem hiding this comment.
The only issue I see with setting the IconButton size to regular is that on hover it looks like this:
Could we work around the positioning of a small IconButton instead? 🤔
6 participants
Minor design updates to the column management panel, including: