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.

Sign up for GitHub

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

Jump to bottom

[DataGrid] Add column reorder support #165

Merged
merged 65 commits into from
Oct 15, 2020
Merged

[DataGrid] Add column reorder support #165

merged 65 commits into from
Oct 15, 2020

Conversation

DanailH
Copy link
Member

@DanailH DanailH commented Aug 12, 2020 •
edited by oliviertassinari
Loading

This PR adds the functionality to reorder data grid columns by dragging and dropping them. It achieves this by utilizing the HTML5 drag and drop API.

The functionality was done by following how similar features were added, e.g useColumnResize. I've added a new hook in the features folder called useColumnResize and inside it is the logic needed for this feature to work.

In addition, I added one CSS class with styles inside the styled-wrappers/grid-root.tsx file.

Unit tests are missing from this PR.

Please leave your feedback regarding the implementation and when we have an agreement I'll add the needed unit tests (and probably documentation)

Closes #194

@dtassone
Copy link
Member

Also I had a look at the UX and I think it needs to be improved.
The scrolling is not smooth and it only works for 1 column
Your draggable div is set to the column title and not to the full width and height of the column header

@oliviertassinari
Copy link
Member

Unit tests are missing from this PR.

I would also include the documentation. We start to have enough infrastructure on the X repository to be able to work incremental with the documentation and tests as part of each pull request, I think that we should leverage it :).

@DanailH
Copy link
Member Author

DanailH commented Aug 17, 2020 •
edited
Loading

Can you guys recheck the PR when you have time?

@DanailH DanailH force-pushed the feature/columns-reorder branch from fb4eb9e to 33f6bd5 Compare August 18, 2020 14:28
@oliviertassinari oliviertassinari mentioned this pull request Aug 19, 2020
Closed
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 19, 2020 •
edited by DanailH
Loading

Opportunities we have identified during yesterday's peer programming session:

  • drag ghost: the drag & drop doesn't complete, which sends the wrong signal to users.
  • missing cursor move, which could help users understand what's going on.
  • scroll horizontally without creating too much mental overhead for end-users.
  • issues with a few of the demos.
  • column width issue with the new div.
  • drag and drop from outside the "header zone", similar to [XGrid] Column resizing is too limiting #191.
  • make sure column virtualization doesn't cause an issue.
  • add documentation.
  • add tests.
  • consider the public API we want to expose for developers to customize the column reordering experience.

@dtassone
Copy link
Member

image
not sure why headers can get this style?

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 10, 2020 •
edited
Loading

All good from my end. I have pushed individual commits for improvement opportunities I could notice (spend about 1 hour on this). Feedback welcomed them.

For the follow-up issues we could open:

  • Drag & drop outside of the header. Allow reordering when inside, cancel reorder when outside.
  • movable CSS lost with virtualization, need to have a declarative API, not an imperative one.
  • Scrolling feels junky, not sure how we can improve the performance.
  • Transition when swapping
  • Allow disabling reorder on one column
  • Allow fixing the position of one column (stronger that disabling reorder on one column)
  • Column group reorder

@DanailH DanailH requested a review from dtassone October 12, 2020 15:05
Danail Hadzhiatanasov and others added 2 commits October 15, 2020 11:19
@DanailH DanailH force-pushed the feature/columns-reorder branch from b81d2a1 to 5274172 Compare October 15, 2020 12:12
@DanailH DanailH merged commit 49237cd into mui:master Oct 15, 2020
@oliviertassinari oliviertassinari mentioned this pull request Oct 15, 2020
Closed
2 tasks
dtassone pushed a commit to dtassone/material-ui-x that referenced this pull request Nov 9, 2020
@DanailH @mbrookes
@oliviertassinari @dtassone
[DataGrid] Add column reorder support (mui#165)
3085f28 
* [useColumnReorder] Add the ability to reorder columns using drag and drop

* [useColumnReorder] Fix review comments

* fix broking functionality

* Fix issues related to pulling the latest changes

* {useColumnReorder] Refactor the feature hook and add drag scroll support for the ColumnsHeader

* [useColumnReorder] Fix lint and format issues

* [useColumnsReorder] Fix build and formatting

* [useColumnReorder] Fix lint errors

* Fix styles downgrade when dragging a column cell

* [useColumnReorder] Added option to disable the feature, added docs, added tests

* [useColumnReorder] Fix formatting and enable all the tests

* Remove unnessesary code from the mouse.test.ts file

* Fix formatting

* fix formatting

* Emit event when drag enters a column

* Working on PR comments

* Fix inifinte loop when dragging smaller cal over large col

* Rename drag handlers prefix

* Prep codebase for commit and push

* Run prettier

* Fix lint errors

* Rename file to match coding style

* Rename columnsHeaderRef to ref

* Fix PR comments

* Update docs/pages/api-docs/x-grid.md

Co-authored-by: Matt <[email protected]>

* Update docs/pages/api-docs/x-grid.md

Co-authored-by: Matt <[email protected]>

* Update docs/pages/api-docs/x-grid.md

Co-authored-by: Matt <[email protected]>

* Add relevant documentation about coumn reorder on the /components/data-grid/columns/ page

* Fix docs formatting

* Update docs/src/pages/components/data-grid/columns/columns.md

Co-authored-by: Olivier Tassinari <[email protected]>

* Update docs/src/pages/components/data-grid/columns/columns.md

Co-authored-by: Olivier Tassinari <[email protected]>

* Update docs/src/pages/components/data-grid/columns/columns.md

Co-authored-by: Olivier Tassinari <[email protected]>

* Update docs/src/pages/components/data-grid/columns/columns.md

Co-authored-by: Olivier Tassinari <[email protected]>

* Update code example

* Fix PR comments

* Fix the doc example code

* Update docs/src/pages/components/data-grid/columns/ColumnOrderingGrid.tsx

Co-authored-by: Olivier Tassinari <[email protected]>

* Fix column reordering issue while dragging over the same cell

* Make scrolling while dragging excelerate

* Format and lint the changes

* Fix scroll left issue

* Fix build

* fix scroll smoothness

* Fix Col header item alignment visuall regression

* Final polishing of the useColumnReorder hook

* Fix typings

* the logic never runs on Node.js

* clear timeout when unmounting, avoid edge-case leak

* use design token

* no shorthand

* Use CSS inherit

* simpler class name logic

* we have strong constraint that these value should be defined. It should fail if its not the case

* no shorthands

* the logic depends on the element to be the current target, not the target, remove potential confusion

* add visual clue about where the column is, outline shouldn't be visible

* Remove unnecessary checks related to disableColumnReorder flag

* Fix PR comments related

* remove raf

Co-authored-by: Matt <[email protected]>
Co-authored-by: Olivier Tassinari <[email protected]>
Co-authored-by: Danail Hadzhiatanasov <[email protected]>
@oliviertassinari oliviertassinari mentioned this pull request Mar 16, 2021
Closed
@oliviertassinari oliviertassinari mentioned this pull request Dec 1, 2022
Closed
2 tasks
@oliviertassinari oliviertassinari added the feature: Reordering Related to the data grid Reordering feature label Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbrookes mbrookes mbrookes left review comments

@oliviertassinari oliviertassinari oliviertassinari approved these changes

@dtassone dtassone dtassone approved these changes

Assignees
No one assigned
Labels
component: data grid This is the name of the generic UI component, not the React module! feature: Reordering Related to the data grid Reordering feature new feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[DataGrid] Implement Column reorder
4 participants
@DanailH @dtassone @oliviertassinari @mbrookes