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.

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

Ability to hide rows/cols #480

Merged
merged 27 commits into from
Feb 17, 2022
Merged

Conversation

hangxingliu
Copy link
Contributor

@hangxingliu hangxingliu commented Jan 30, 2022

Related issue #420

And this PR doesn't need to be merged now. I am still adding the UI tests and documents for it. I create a PR first for everyone can have a look now. And you can leave messages here if you find any bugs with this new feature. Thanks

* Add `hiddenRowRanges`

* Update `getFilteredAndSortedViewData` to support hidden rows

* Add public methods `hideRows` and `unhideRows`
* Add method `getSelectedContiguousRows`
* Add `currentRowIndexOffset` and `rowIndexOffsetByHiddenRows` to make the current row index model is compatible with hidden rows.

* Hide the row gap indicator(a bold black line) for the hidden rows.
* And call this method in the `stopDragMove` to fix incorrect values in `scrollCache.x`
@hangxingliu
Copy link
Contributor Author

@ndrsn This commit 9513be0 is used to for fixing this bug:

Screen.Recording.2022-01-31.at.08.51.03.mov

This bug causes hidden columns to look weird when the user moves the frozen marker.

@ndrsn
Copy link
Collaborator

ndrsn commented Feb 8, 2022

@hangxingliu I'm a little tied up this week, but can look early next, unless @TonyGermaneri wants to review it?

@ndrsn
Copy link
Collaborator

ndrsn commented Feb 17, 2022

@hangxingliu, thanks a lot for this — this is great work, and I really appreciate the effort you've put into the tests.

Beyond nitpick-stuff (which I won't mention because it's not important), I found nothing worth mentioning. It all looks pretty good. (Although the only nitpick I might mention would be to add some more more vertical spacing, i.e. empty lines, here and there to separate some of the logical blocks for legibility).

It is, tough, a lot of code, so while I've tried to go through it diligently I may have missed some things. If so: apologies :-)

@ndrsn ndrsn merged commit 50c3f0e into TonyGermaneri:master Feb 17, 2022
@hangxingliu hangxingliu deleted the feature-420-new branch February 17, 2022 06:24
@hangxingliu
Copy link
Contributor Author

@ndrsn Thanks for your appreciation and your work. And you can leave the comments on the code for me if you want. Because sometimes I am focus on the the logic of the code but forget about legibility.

ndrsn pushed a commit to ndrsn/canvas-datagrid that referenced this pull request Feb 23, 2022
* Fix the behavior of the conext menu item used to hide column

* Add new attributes and styles for hide contiguous columns/rows

* Update context menu to support hiding multiple columns

* Update event handlers for unhide indicator

* Rename `strokeLines` to `drawLines`

* Fix unresolved merge conflict

* Add methods and properties for the feature of unhide indicator

* Implement the render of the unhide indicator

* Update the render of unhide indicators on row headers

* Add util function `mergeHiddenRowRanges` and unit tests.

* Update methods and properties for hide/unhide feature

* Add `hiddenRowRanges`

* Update `getFilteredAndSortedViewData` to support hidden rows

* Add public methods `hideRows` and `unhideRows`

* Update context menu for hiding rows

* Add method `getSelectedContiguousRows`

* Update click event handler to support unhide rows

* Update `draw.js` to support hidden rows

* Add `currentRowIndexOffset` and `rowIndexOffsetByHiddenRows` to make the current row index model is compatible with hidden rows.

* Hide the row gap indicator(a bold black line) for the hidden rows.

* Update `draw.js` for unhide indicator

* Fix jsDoc

* Extract a new method `refreshScrollCacheX` from the method `resize`

* And call this method in the `stopDragMove` to fix incorrect values in `scrollCache.x`

* Fix `Cannot read 'context' of undefined`

* Add method `hideColumns`

* Fix the relationship between unhide indicator and dragging

* Fix generating codes of`visibleUnhideIndicators`

* Fix incorrect indexes in row headers when the first row is hidden

* Add tests for unhide indicator

* Add util functions for tests

* Fix tests for unhide indicator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants