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

[usePinColumns] Hook to pin/freeze columns #1510

Closed
wants to merge 2 commits into from

Conversation

gargroh
Copy link
Contributor

@gargroh gargroh commented Sep 11, 2019

No description provided.

@tannerlinsley
Copy link
Collaborator

Wow. I have to say, this is a fantastic PR. Seriously. One of the best I've ever received for React Table.

Can you start working on the docs?

@gargroh
Copy link
Contributor Author

gargroh commented Sep 12, 2019

Thank You @tannerlinsley for appreciating, it will bring more motivation to me.

Updated docs.

@tannerlinsley
Copy link
Collaborator

tannerlinsley commented Sep 12, 2019

Yeah, this is looking really good. Next steps would be to:

  • Create an example to demonstrate the functionality
  • Create at the very least a snapshot-driven test for the plugin-hook

You have me thinking now when it comes to the Example section in the docs... all of them should just link to real examples. Thoughts?

@gargroh
Copy link
Contributor Author

gargroh commented Sep 12, 2019

Yes, removing code from docs will make them cleaner, anyways we are having working examples for each functionality.

@gargroh
Copy link
Contributor Author

gargroh commented Sep 16, 2019

Examples for these will make more sense with useAbsoluteLayout.

@tannerlinsley
Copy link
Collaborator

Examples for these will make more sense with useAbsoluteLayout.

Why is that exactly?

@gargroh
Copy link
Contributor Author

gargroh commented Sep 17, 2019

My bad absolute layout, is not necessary for basic implementation of pinning of columns, implemented an example of usePinColumns at https://codesandbox.io/s/tannerlinsleyreact-table-column-ordering-jsxs5 (https://jsxs5.csb.app/). Got to see one more case needs to be handled in case of pinning that is of grouped headers's parent pinType. This needs to be identified where this change has to be done, because before preparation of columns parent remains to be same and have same data.

@gargroh
Copy link
Contributor Author

gargroh commented Sep 24, 2019

Hey @tannerlinsley , whenever you get a chance, please look into above and also #1522

@tannerlinsley
Copy link
Collaborator

This is also looking really good. Yeah that one case where the column has parent header groups is kinda wonky. Do you have an updated example that fixes that?

@gargroh
Copy link
Contributor Author

gargroh commented Sep 27, 2019

Do you have an updated example that fixes that?

Not sure that this can fixed via example only, it seems that some changes should be required in useTable hook, so that the same parent object can be treated in two different ways if table data columns are re-ordered.

@tannerlinsley
Copy link
Collaborator

After giving this one some more thought, I think what this is actually doing is creating "sticky" columns... and that can get tricky really fast. There are at least a few ways to implement sticky columns in a table and I think I need to look into this more. I would rather not have to have the user build 3 separate tables elements to do this. BRB

@tannerlinsley
Copy link
Collaborator

Here is a little codesandbox I'm playing around with: https://codesandbox.io/s/tannerlinsleyreact-table-column-ordering-qupnl

Essentially, this plugin is simply a different variation of the useColumnOrder hook. It's in combination with sticky styles that it becomes a true column "pin". I think we should move forward with this by discovering the best way to make "sticky" columns portable. This way you could combine the sticky capabilities with the pinning capability more easily.

Sticky is interesting. You can easily do sticky pinning on your own for a single column on either end, but any time you get multiple sticky columns that you want to "stack" up, you'll need some way to detect the size of the columns and programmatically set the left or right sticky positioning, eg. left: ${totalWidthToTheLeftOfMe}px`

Honestly, I don't think that would be too difficult. All you would need to do is introduce a layoutEffect that can measure the dom before it paints and store the rendered column sizes in state.

This seems to be rabbit hole-ing though. Thoughts?

@tannerlinsley
Copy link
Collaborator

After some more thought, I am pretty sure I'd like to see this hook basically become an alternative to the useColumnOrder hook. Essentially, that's what it's doing at it's core is just rearranging columns based on a pin direction. If you cut off it's features there, then it's awesome as is.

Let's get the example tuned up to not do any fancy pinning or anything like that, just have it render the table normally (the columns will be rearranged)

Then we can handle the "sticky" stuff (pun intended) in another hook.

@gargroh
Copy link
Contributor Author

gargroh commented Oct 4, 2019

This seems to be rabbit hole-ing though. Thoughts?

The reason why i created it with three segments was that each segment can have multiple columns and developers can define segments's minWidth maxWidth etc. so as to have each segment their scrollbar. And creating segments dont require any extra markup, the same header+row component can serve the purpose with some extra checks.

Let's get the example tuned up to not do any fancy pinning or anything like that, just have it render the table normally (the columns will be rearranged)

https://codesandbox.io/s/tannerlinsleyreact-table-column-ordering-ttc3s

Then we can handle the "sticky" stuff (pun intended) in another hook.

Agree, later on we can provide a basic example with sticky, lets not cover that with this PR. Moreover, as the html structure is fully controlled by lib's user, so if they want they can create whatever suits them better.

@gargroh
Copy link
Contributor Author

gargroh commented Nov 14, 2019

@tannerlinsley , after analyzing the problem, only column re-ordering will not solve this, we need to have different headerGroups also for each segment 'left', center and 'right'...

For example, say Name is grouped header for First Name and Last Name, then if user has pinned Last Name then this lName column should have a parent header called Name and also fName should have another parent header Name. Agree on this part ?

@gargroh
Copy link
Contributor Author

gargroh commented Nov 14, 2019

I have written a following revamp of usePinColumns, do let me know your opinion -

https://gist.github.com/gargroh/ab11066424a0bdc775fe0f7eeb451a16

@tannerlinsley
Copy link
Collaborator

Yes I believe you are correct about the header groups. Pinning columns would mean you need to first reorder the headers, then regroup them. This is why it needs to be done in the earliest hook. Reordering the headers. Like I said, try duplicating the setup of the useColumnOrder plugin, but with your left/center/right logic instead. Once that is done, we can merge it and continue on to work on the "sticky" part of the problem... pun intended.

@gargroh
Copy link
Contributor Author

gargroh commented Nov 21, 2019

I have written a following revamp of usePinColumns, do let me know your opinion -

https://gist.github.com/gargroh/ab11066424a0bdc775fe0f7eeb451a16

@tannerlinsley any thoughts about it ?

@tannerlinsley
Copy link
Collaborator

It's a bit better, but I still think that it needs to hook into the column order earlier so that the core can do all of the header rebuilding automatically.

I would suggest looking at the new refactor for reducers and also how the column ordering hook works, then open a new WIP PR for discussion around this.

Going to close this one for now. Let me know if you want to reopen with new code.

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

2 participants