ted-marozzi / data-grid

High Performance Data Grid optimised for large numeric tables.
BSD 3-Clause "New" or "Revised" License
8 stars 4 forks source link

feat: allow rows without headers #12

Closed defuncart closed 2 years ago

defuncart commented 2 years ago

This PR adds a couple of features which are useful on desktop:

Also added a theme for the widget.

Bildschirmfoto 2022-11-10 um 13 12 36

Highlighting and selected works well when there is no row header. In the case of row header, this is disabled.

ted-marozzi commented 2 years ago

image Looks good, only thing is the column header is still frozen. Also could you document the highlighting issue with header rows so users will know. I'd also like to see what the issue is so we can try to fix it

defuncart commented 2 years ago

only thing is the column header is still frozen.

I think this should be a separate feature, and is something I reckon most users would expect enabled by default. The hasRowHeader boolean is so that there are no frozen row headers.

Also could you document the highlighting issue with header rows so users will know.

Added some basic documentation, let me know if it could be improved.

I'd also like to see what the issue is so we can try to fix it

Updated the code to cover both cases. This is achieved by syncing between GridRowHeader and GridRows which index is hovered.

ted-marozzi commented 2 years ago

Sorry I wasn't clear, hasRowHeader is false, the cell at x,y 0,0 should not let the cell at x,y 0,1 go behind it. Do you disagree with that?

ted-marozzi commented 2 years ago

Thanks for the docs, and great!

defuncart commented 2 years ago

Sorry I wasn't clear, hasRowHeader is false, the cell at x,y 0,0 should not let the cell at x,y 0,1 go behind it. Do you disagree with that?

Sorry, missed that 🙈 Updated PR :)

defuncart commented 2 years ago

While merging #12 and #14 on a local branch, there are some merge conflicts and I think it is better to merge #12 and then #14 into master, as the solution to #14 should be updated once #12 is merged. Thus marking #14 as draft.