ember-light-table icon indicating copy to clipboard operation
ember-light-table copied to clipboard

[WIP] Fixed columns

Open cball opened this issue 8 years ago • 15 comments

This has a decent amount more for me to do before I'd consider it mergeable but I wanted to open it up for discussion.

This library is gaining traction, so I wanted to look at adding some of the high level features developed in https://github.com/cball/justa-table so that I can put resources towards this library instead. We currently have 3 projects that need fixed column support and I know others that have expressed the need for that feature as well. @taras had concerns using fixed columns in https://github.com/offirgolan/ember-light-table/issues/114. While complicated to implement, it is a pretty common ask for complex tables and I don't think you can always just implement a different UI.

First, the largest issue: Trackpad Scroll Emulator which is used by ember-scrollable can only scroll 1 way. (see https://github.com/jnicol/trackpad-scroll-emulator#5-limitations). As such, in this PR when there are fixed columns present, native scrollbars are used instead. I'd actually argue to put the work in to make native scrollbars work across the board but that's a separate issue.

TODOs before I think this is ready:

  • [ ] fix and support columnGroups
  • [ ] fix ember-infinity
  • [ ] add missing tests for new functionality
  • [ ] reduce duplication (currently a decent amount of copy/paste) to set up the columns
  • [ ] figure out a better way to organize the fixed column specific code
  • [ ] general cleanup

screen recording 2016-07-29 at 09 10 pm

cball avatar Jul 30 '16 01:07 cball

@cball This is.. this is awesome! Im super excited for this. I think we are gonna have to make the font size on the navbar smaller to fit all these features in 😏

From a quick glance, I noticed that you're branch is a bit behind master. I would suggest to rebase since it has a lot of changes that resolve some critical issues and that will prolly conflict with some of your code.

In terms of the trackpad scroll emulation, I think @taras would be your man.

offirgolan avatar Jul 30 '16 07:07 offirgolan

@cball this is awesome!

I ended up adding Trackpad Scroll Emulation because we have fixed headers and footers which are setup as separate tables. With regular scrollbars, the columns in the header didn't line up with the body because they had different widths, caused by scrollbar being added to the body.

I'd love a simpler solutions. It'd mean that we'd be able to remove a dependency and reduce complexity. If you have any suggests in regards to how to manage the content width issue caused by body scrollbar, I'd love to hear them.

taras avatar Jul 30 '16 08:07 taras

Thanks! I'll rebase and get this finished / cleaned up.

In the first pass of this feature, I want to keep the existing virtual scroll behavior in tables without fixed columns. We should be able to normalize the scroll behaviors in all tables with a followup PR. tldr there I think will be measuring scrollbars and explicitly setting widths.

We should also be able to simplify the codebase a bit in the future by making all tables without fixed columns operate the same as the standard columns from the fixed version.

cball avatar Jul 30 '16 20:07 cball

tldr there I think will be measuring scrollbars and explicitly setting widths.

I'm concerned about this. So far, I've been very intentional about not getting into a situation where we're managing dimensions of elements through state and rely on the browser to size elements appropriately.

Mixing state managed sizes and DOM sizes quickly leads to having to manage all sizes through state. This is part of the reason why we're using ember-element-resize-detector. Instead of forcing nested element to have a height, we're detecting the size of the parent and resizing the child as necessary.

I would prefer for us to continue to evolve this pattern rather than getting into setting widths and heights via state. I'm going to work on a prototype that has nested ember-scrollable components. If I can get that to work, then I'll try to build a prototype that shows how we might be able to implement this feature.

My other concern is that collapsible rows will not work with fixed columns(without doing some complex height management). This is the reason for my comment in #114. I was hoping we'd be able to use something like ember-collection where we could create a grid that was fixed columns aware. ember-collection never materialized unfortunately.

taras avatar Jul 30 '16 21:07 taras

It doesn't look like nested ember-scrollable approach is going to work. I might have to make ember-scrollable support vertical and horizontal scrollbars.

I'll keep you posted on my progress.

In the meantime, I'm curious what your thoughts are regarding collapsible rows and fixed columns.

taras avatar Jul 30 '16 22:07 taras

I'm concerned about this. So far, I've been very intentional about not getting into a situation where we're managing dimensions of elements through state and rely on the browser to size elements appropriately.

Ember scrollable is already measuring the scrollbar and setting widths/heights, not sure how is this different?

Fixed columns are implemented using 2 absolute positioned containers with separate tables, as such they need widths and heights set.

I can certainly use ember-element-resize-detector to set the height of the absolutely positioned containers, but will still need to set the fixed column container width based on the sum of all fixed columns, and set the margin-left of the standard columns equal to that same amount. I don't see a way around explicitly setting that.

Re: collapsable rows, yes we'll probably have to do some height management so things don't get out of sync. I think I'd probably start by firing an action from the lt-spanned-row component when it has finished loading its content and has a height. Row is already an object proxy, so perhaps we can drive the syncing of height that way. I'll can update my dummy app example to include that and explore what makes sense.

High level question: Should an expanded row take on the width of the entire table (both fixed and standard columns) or just the non-fixed side?.

Agree its too bad about ember-collection, I was looking forward to that as well.

cball avatar Aug 01 '16 18:08 cball

This is a very exciting feature, this behavior could be very useful in regard to expanding the responsive design aspects of this table component.

nmcclay avatar Sep 30 '16 13:09 nmcclay

Where does this feature currently stand? I'd be glad to help push it along

RobbieTheWagner avatar Dec 15 '16 16:12 RobbieTheWagner

@rwwagner90 I was waiting on feedback on how to proceed. I unfortunately don't have any free cycles at the moment, but if this is a direction that @offirgolan and @taras would like to go with this library (I'm unsure if it is) then help moving things along would be greatly appreciated.

cball avatar Dec 20 '16 21:12 cball

Is there any progress with the PR? I have been waiting for this feature to be merged.

Prabhakar-Poudel avatar Jan 09 '17 04:01 Prabhakar-Poudel

@offirgolan @taras can you guys weigh in here please? I would be glad to help with implementing this, but do not want to put effort in, if it is not something that is desired for this addon.

RobbieTheWagner avatar Jan 09 '17 18:01 RobbieTheWagner

This PR has been in a bit of standstill mostly because we're not sure the best way to proceed. We would like for this feature to happen, but we weren't 100% what approach to take. @cball proposed a solution, but we inadvertently blocked him by not providing a path for moving forward.

@alexander-alvarez has a PR in the works for ember-scrollable that'll make it possible to have horizontal and vertical scrollbars on a block. Once this lands, it might unblock this PR and provide a way to implement this feature without making the code a lot more complicated.

Would you be interested in picking this up when that PR lands?

Primarily, we need to figure out how the solution proposed in this PR is affected by that change. ember-collection is also becoming much more robust and approaching 1.0. Now might be a good time to see if we can use it to implement this feature.

What do you think?

taras avatar Jan 09 '17 20:01 taras

has there been there any progress with this?

Prabhakar-Poudel avatar Feb 02 '17 09:02 Prabhakar-Poudel

It looks like the ember-scrollable PR has been merged. After a year, is there any news on this feature?

gspain avatar Aug 30 '18 18:08 gspain

@cball @taras @offirgolan I'm guessing this PR will probably need a rebase and some reworking, but now that ember-scrollable has what is needed, are there any blockers left to getting this PR in?

I think it would be a very popular feature - tables being responsive and usable at small viewports is a huge deal!

Techn1x avatar May 20 '19 04:05 Techn1x