[XGrid] Support column reordering inside the whole grid #1250
Conversation
|
I need some help to fix the failing tests on CI. In all browsers they passed, but not on Safari. Locally, using the version I have, they passed too.
|
| @@ -102,6 +101,21 @@ export const GridCell: React.FC<GridCellProps> = React.memo((props) => { | |||
| [publish, publishClick], | |||
| ); | |||
|
|
|||
| const onDragEnter = React.useCallback((event) => apiRef!.current.onColItemDragEnter(event), [ | |||
There was a problem hiding this comment.
this onColItem... should be refactored and will be removed
#1201
There was a problem hiding this comment.
I think we still need to keep the APIs because they are used in the header cells but we will need to add handlers on the grid component itself that call those APIs to satisfy the requirement from #1201. We should change the names tho to match convention, e.g. handleDragEnd for example.
There was a problem hiding this comment.
apiRef should not be a trigger for events.
There should not be any method like handleDragEnd on it. You can subscribe to an handler using subscribeEvent but not just put them there. That's not its purpose.
Only api methods that can be used to retrieve data, or manipulate the grid.
Something like setColumnIndex(field, index) is appropriate in this case.
So users can reset the index outside the drag action.
| @@ -52,6 +50,7 @@ export const GridCell: React.FC<GridCellProps> = React.memo((props) => { | |||
| const valueToRender = formattedValue || value; | |||
| const cellRef = React.useRef<HTMLDivElement>(null); | |||
| const apiRef = React.useContext(GridApiContext); | |||
| const { align = 'left', field } = column; | |||
There was a problem hiding this comment.
Why do you need the column now?
There was a problem hiding this comment.
I need it to call onColItemDragOver later in this file. Previously I just had the field and align from the column as props.
There was a problem hiding this comment.
in #1201 we will remove this method of apiRef as they don't really make sense.
I think #1201 should come before this changes.
This method start an action instead of letting you subscribe to an event.
You can refactor by passing the field and retrieving the column directly in the handler using apiRef.
|
lot of refactoring are also coming on the grid cell as part of #1205 |
m4theushw
force-pushed
the
column-reorder
branch
from
478d627 to
3adbea6
Compare
oliviertassinari
added
components: XGrid
new feature
@m4theushw You can use our BrowserStack credentials to run the test in Safari, only. Here https://www.notion.so/BrowserStack-945f0ccb9f7749ccb9e84b9eb9b13ec7, use the "env" part. |
m4theushw
force-pushed
the
column-reorder
branch
from
3adbea6 to
1325e6e
Compare
| @@ -59,12 +67,6 @@ export const GRID_COL_RESIZE_STOP = 'colResizing:stop'; | |||
|
|
|||
| export const GRID_COLUMN_ORDER_CHANGE = 'columnOrderChange'; | |||
|
|
|||
| export const GRID_COLUMN_REORDER_START = 'columnReordering:dragStart'; | |||
There was a problem hiding this comment.
IMO events should be named according to the DOM events that they listen to and not the context in which they are used. Different hooks could subscribe to the same event.
Fixes #445.
Preview https://deploy-preview-1250--material-ui-x.netlify.app/components/data-grid/demo/
Breaking changes
GRID_COLUMN_REORDER_DRAG_OVER_HEADERis no longer emittedMy approach here was to turn every cell into a droppable target. If #206 gets implemented an additional check may be needed to determine if a column cell or a row cell was dropped. I'm using the value of
dropEffectto determine if the user canceled the reordering (dropping outside or pressing Escape).Result: