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

useAbsoluteLayout: Enable react-table to build with divs #1522

Merged
merged 7 commits into from Sep 30, 2019

Conversation

gargroh
Copy link
Contributor

@gargroh gargroh commented Sep 16, 2019

Inspired by useFlexLayout.
Enable react-table to build with divs
Help in integrating virtual scrolling

Disclaimer: May require some fixes(to integrate well with column order/pinning)

@tannerlinsley
Copy link
Collaborator

Very cool! Let's get a few other things going so we can follow along better:

  • A codesandbox demonstrating the plugin (you can upload it as the example, too)
  • Documentation
  • At the very least, some snapshot tests (this can come last)

@tannerlinsley
Copy link
Collaborator

It's also worth noting that a resizeColumns hook would technically work better with an absolute layout like this, instead of with a table.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 19, 2019

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6bcd42b:

Sandbox Source
admiring-cherry-dvskr Configuration

@gargroh
Copy link
Contributor Author

gargroh commented Sep 20, 2019

@tannerlinsley , good to merge form my side, have a look when you get a chance.

@tannerlinsley
Copy link
Collaborator

This is looking really really good. One thing that I think we could expect from a plugin like this is that rows would also be absolutely positioned. What do you think?

I know that would required that rowHeights be defined and everything. And obviously, if it wasn't we could support a relative row position like it is right now.

Thoughts?

@gargroh
Copy link
Contributor Author

gargroh commented Sep 27, 2019

My thoughts are - absolute positioned rows will be useful in case of virtual scrolling only, which if anybody wants(which most of them will) can use any virtualization library(like react-window etc) for it and provide the inputs needed by it(whether the grid is of variable row heights), putting that functionality in this hook will be a redundant one in my opinion.

@tannerlinsley
Copy link
Collaborator

Yeah, that's a good point. Okay, I think I'm ready to merge this.

@tannerlinsley tannerlinsley merged commit b6fdb99 into TanStack:master Sep 30, 2019
@stramel
Copy link
Contributor

stramel commented Sep 30, 2019

IMO this seems like an unnecessary plugin to pull into this library. I thought one of the benefits to this plugin system, was you could write your own plugin hook without having to get it into this lib. Seems like it could be published as a 3rd party plugin and could be listed as an option somewhere on the README?

Also, @tannerlinsley, this seems to be referring to a plugin that doesn't exist? usePinColumns?

@tannerlinsley
Copy link
Collaborator

The way I see it, this hook is similar in spirit to the much needed useFlexLayout hook, which is not done yet. But, both of these provide some good ground work for a few things that are currently not possible without divs:

  • Row virtualization
  • Resizable columns
  • Sticky columns

This hook doesn't alter the core in any way, is entirely tree-shakeable and should be extremely simple to maintain with the rest of the library. If it presents problems in the future, we can always move it out to its own import or repo, but I don't want to make that decision before it's needed.

@tannerlinsley
Copy link
Collaborator

usePinColumns is still a WIP hook (has a PR opened currently) that may or may not make it into the core. It may even be repurposed as a useStickyColumns hook. We'll have to see.

@tannerlinsley
Copy link
Collaborator

tannerlinsley commented Oct 1, 2019

I'd actually like to revisit this idea and possibly make a few changes:

  • Internalize the logic for width etc measurements into the core and support them at the column level as first-class options
  • Change this hook's responsibility to simply applying those measurements as absolute layout

This is in light of other layout hooks that could be built using the same technique as this one. Mainly, I'm thinking about the useFlexLayout hook.

@gargroh gargroh deleted the patch-6 branch May 2, 2020 15:04
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.

None yet

3 participants