-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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] Store the outlined cell in the state #7111
Changes from 8 commits
4babd8b
3199aea
55686c7
7623bd9
00bffba
70cd4d4
9b36e02
ee400ee
7535fc0
297b7ca
c7ea049
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -200,6 +200,11 @@ The minimum supported Node.js version has been changed from 12.0.0 to 14.0.0, si | |||||||||||
|
||||||||||||
### CSS classes | ||||||||||||
|
||||||||||||
- To update the outline style of a focused cell, use the `.MuiDataGrid-cell--outlined` class instead of the `:focus-within` selector. | ||||||||||||
```diff | ||||||||||||
-'.MuiDataGrid-cell:focus-within': { | ||||||||||||
+'.MuiDataGrid-cell--outlined': { | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Maybe include an example with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added! |
||||||||||||
``` | ||||||||||||
- Some CSS classes were removed or renamed | ||||||||||||
|
||||||||||||
| MUI X v5 classes | MUI X v6 classes | Note | | ||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,6 @@ import { | |
import { GridAlignment } from '../../models/colDef/gridColDef'; | ||
import { useGridApiContext } from '../../hooks/utils/useGridApiContext'; | ||
import { useGridRootProps } from '../../hooks/utils/useGridRootProps'; | ||
import { gridFocusCellSelector } from '../../hooks/features/focus/gridFocusStateSelector'; | ||
import { DataGridProcessedProps } from '../../models/props/DataGridProps'; | ||
import { FocusElement } from '../../models/params/gridCellParams'; | ||
|
||
|
@@ -32,6 +31,7 @@ export interface GridCellProps<V = any, F = V> { | |
hasFocus?: boolean; | ||
height: number | 'auto'; | ||
isEditable?: boolean; | ||
isOutlined?: boolean; | ||
isSelected?: boolean; | ||
showRightBorder?: boolean; | ||
value?: V; | ||
|
@@ -65,17 +65,21 @@ function doesSupportPreventScroll(): boolean { | |
return cachedSupportsPreventScroll; | ||
} | ||
|
||
type OwnerState = Pick<GridCellProps, 'align' | 'showRightBorder' | 'isEditable' | 'isSelected'> & { | ||
type OwnerState = Pick< | ||
GridCellProps, | ||
'align' | 'showRightBorder' | 'isEditable' | 'isOutlined' | 'isSelected' | ||
> & { | ||
classes?: DataGridProcessedProps['classes']; | ||
}; | ||
|
||
const useUtilityClasses = (ownerState: OwnerState) => { | ||
const { align, showRightBorder, isEditable, isSelected, classes } = ownerState; | ||
const { align, isOutlined, showRightBorder, isEditable, isSelected, classes } = ownerState; | ||
|
||
const slots = { | ||
root: [ | ||
'cell', | ||
`cell--text${capitalize(align)}`, | ||
isOutlined && `cell--outlined`, | ||
isEditable && 'cell--editable', | ||
isSelected && 'selected', | ||
showRightBorder && 'cell--withRightBorder', | ||
|
@@ -87,8 +91,6 @@ const useUtilityClasses = (ownerState: OwnerState) => { | |
return composeClasses(slots, getDataGridUtilityClass, classes); | ||
}; | ||
|
||
let warnedOnce = false; | ||
|
||
function GridCell(props: GridCellProps) { | ||
const { | ||
align, | ||
|
@@ -101,6 +103,7 @@ function GridCell(props: GridCellProps) { | |
hasFocus, | ||
height, | ||
isEditable, | ||
isOutlined, | ||
isSelected, | ||
rowId, | ||
tabIndex, | ||
|
@@ -118,6 +121,8 @@ function GridCell(props: GridCellProps) { | |
onMouseUp, | ||
onMouseOver, | ||
onKeyDown, | ||
onFocus, | ||
onBlur, | ||
onKeyUp, | ||
onDragEnter, | ||
onDragOver, | ||
|
@@ -130,7 +135,14 @@ function GridCell(props: GridCellProps) { | |
const apiRef = useGridApiContext(); | ||
|
||
const rootProps = useGridRootProps(); | ||
const ownerState = { align, showRightBorder, isEditable, classes: rootProps.classes, isSelected }; | ||
const ownerState = { | ||
align, | ||
showRightBorder, | ||
isEditable, | ||
classes: rootProps.classes, | ||
isSelected, | ||
isOutlined, | ||
}; | ||
const classes = useUtilityClasses(ownerState); | ||
|
||
const publishMouseUp = React.useCallback( | ||
|
@@ -208,36 +220,6 @@ function GridCell(props: GridCellProps) { | |
} | ||
}, [hasFocus, cellMode, apiRef]); | ||
|
||
let handleFocus: any = other.onFocus; | ||
|
||
if ( | ||
process.env.NODE_ENV === 'test' && | ||
rootProps.experimentalFeatures?.warnIfFocusStateIsNotSynced | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This flag is useless now because when a cell is focused via keyboard, |
||
) { | ||
handleFocus = (event: React.FocusEvent) => { | ||
const focusedCell = gridFocusCellSelector(apiRef); | ||
if (focusedCell?.id === rowId && focusedCell.field === field) { | ||
if (typeof other.onFocus === 'function') { | ||
other.onFocus(event); | ||
} | ||
return; | ||
} | ||
|
||
if (!warnedOnce) { | ||
console.warn( | ||
[ | ||
`MUI: The cell with id=${rowId} and field=${field} received focus.`, | ||
`According to the state, the focus should be at id=${focusedCell?.id}, field=${focusedCell?.field}.`, | ||
"Not syncing the state may cause unwanted behaviors since the `cellFocusIn` event won't be fired.", | ||
'Call `fireEvent.mouseUp` before the `fireEvent.click` to sync the focus with the state.', | ||
].join('\n'), | ||
); | ||
|
||
warnedOnce = true; | ||
} | ||
}; | ||
} | ||
|
||
const column = apiRef.current.getColumn(field); | ||
const managesOwnFocus = column.type === 'actions'; | ||
|
||
|
@@ -277,10 +259,11 @@ function GridCell(props: GridCellProps) { | |
onMouseDown={publishMouseDown('cellMouseDown')} | ||
onMouseUp={publishMouseUp('cellMouseUp')} | ||
onKeyDown={publish('cellKeyDown', onKeyDown)} | ||
onBlur={publish('cellBlur', onBlur)} | ||
onFocus={publish('cellFocus', onFocus)} | ||
onKeyUp={publish('cellKeyUp', onKeyUp)} | ||
{...draggableEventHandlers} | ||
{...other} | ||
onFocus={handleFocus} | ||
> | ||
{renderChildren()} | ||
</div> | ||
|
@@ -304,6 +287,7 @@ GridCell.propTypes = { | |
hasFocus: PropTypes.bool, | ||
height: PropTypes.oneOfType([PropTypes.oneOf(['auto']), PropTypes.number]).isRequired, | ||
isEditable: PropTypes.bool, | ||
isOutlined: PropTypes.bool, | ||
isSelected: PropTypes.bool, | ||
onClick: PropTypes.func, | ||
onDoubleClick: PropTypes.func, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -124,7 +124,7 @@ export const GridRootStyles = styled('div', { | |
padding: '0 10px', | ||
boxSizing: 'border-box', | ||
}, | ||
[`& .${gridClasses.columnHeader}:focus-within, & .${gridClasses.cell}:focus-within`]: { | ||
[`& .${gridClasses.columnHeader}:focus-within, & .${gridClasses['cell--outlined']}`]: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we ditch There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to do this change now. Creating a |
||
outline: `solid ${ | ||
theme.vars | ||
? `rgba(${theme.vars.palette.primary.mainChannel} / 0.5)` | ||
|
@@ -133,7 +133,7 @@ export const GridRootStyles = styled('div', { | |
outlineWidth: 1, | ||
outlineOffset: -1, | ||
}, | ||
[`& .${gridClasses.columnHeader}:focus, & .${gridClasses.cell}:focus`]: { | ||
[`& .${gridClasses.columnHeader}:focus, & .${gridClasses['cell--outlined']}`]: { | ||
outline: `solid ${theme.palette.primary.main} 1px`, | ||
}, | ||
[`& .${gridClasses.columnHeaderCheckbox}, & .${gridClasses.cellCheckbox}`]: { | ||
|
@@ -353,10 +353,6 @@ export const GridRootStyles = styled('div', { | |
display: 'flex', | ||
boxShadow: theme.shadows[2], | ||
backgroundColor: (theme.vars || theme).palette.background.paper, | ||
'&:focus-within': { | ||
cherniavskii marked this conversation as resolved.
Show resolved
Hide resolved
|
||
outline: `solid ${(theme.vars || theme).palette.primary.main} 1px`, | ||
outlineOffset: '-1px', | ||
}, | ||
}, | ||
[`& .${gridClasses['row--editing']}`]: { | ||
boxShadow: theme.shadows[2], | ||
|
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.
Do we still need
cellFocusIn
/cellFocusOut
events?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.
Yes, for editing.
cellBlur
is tied to theblur
DOM event, meaning that it's published every time a cell loses focus, including when the focus is lost to a portaled element inside the same cell. In the other hand,cellFocusOut
is only published when focus state is updated. If a cell loses focus to an portaled element inside it,cellFocusOut
is not published butcellBlur
is.