daniel-nagy / fixed-table-header

Fixed table header directive.
MIT License
88 stars 36 forks source link

Implemented with Angular Material's $mdSticky service #15

Open jraadt opened 8 years ago

jraadt commented 8 years ago

Implemented fixed-table-header using Angular Material's $mdSticky service. Basically combined daniel-nagy's fixed-table-header with the ideas from Angular Material's md-subheader. Moved much of the logic out of the compile function of the directive and into the link function, which fixed many compiling and transclusion issues. There's no longer a restriction to have the table wrapped in a element with a specified height. It works great with just md-content wrapper. Fixes issues rendering in certain cases with ui-router, ng-if, ng-repeat, etc. I believe this fixes issues #7, #11, #12, and #13.

daniel-nagy commented 8 years ago

@jraadt I'm not sure I want to make ngMaterial a dependency. There may be folks using this without Angular Material. I think we should write our own service so more people can benefit from this.

I'm not sure what the $mdSticky service does, I'll take a look at it this afternoon.

jraadt commented 8 years ago

@daniel-nagy I agree. As I was checking it in and adding $mdSticky to the injected items I realized that it didn't depend on Material. For some reason I just assumed it did because I came from your md-data-tables world. I'm guessing many people using this are in the same boat so maybe this helps them for now. Long term the pieces/ideas from $mdSticky could be pulled into this and no longer have a hard dependency on $mdSticky.

epelc commented 8 years ago

Note if anyone is trying to use this make sure you do not pull in the minified js file(it's still the original).

jraadt commented 8 years ago

@epelc Good catch...rookie mistake. I've pushed up a minified version now.

epelc commented 8 years ago

@jraadt are you on gitter or irc or something? I'm trying to get it to work in my project now but I'm running into errors.

element.parent().parent()[0].insertBefore(stickyPlaceholder[0], element.parent()[0]);

angular.js:13550 TypeError: Cannot read property 'insertBefore' of undefined
    at link (https://secrethost/app/vendor.js:1431:36)
    at ja (https://ajax.googleapis.com/ajax/libs/angularjs/1.5.5/angular.min.js:80:350)
    at n (https://ajax.googleapis.com/ajax/libs/angularjs/1.5.5/angular.min.js:66:28)
    at g (https://ajax.googleapis.com/ajax/libs/angularjs/1.5.5/angular.min.js:58:305)
    at n (https://ajax.googleapis.com/ajax/libs/angularjs/1.5.5/angular.min.js:65:473)
    at g (https://ajax.googleapis.com/ajax/libs/angularjs/1.5.5/angular.min.js:58:305)
    at https://ajax.googleapis.com/ajax/libs/angularjs/1.5.5/angular.min.js:57:455
    at https://secrethost/app/vendor.js:1494:27
    at https://ajax.googleapis.com/ajax/libs/angularjs/1.5.5/angular.min.js:57:445
    at https://ajax.googleapis.com/ajax/libs/angularjs/1.5.5/angular.min.js:62:281 <thead md-head="" fix-head="" class="md-head ng-isolate-scope">

Have you tried it with angular 1.5.x(I'm using 1.5.5 and angular-material 1.0.8)?

@jraadt It doesn't seem to work with angular 1.4.8 and angular-material 1.0.0 or with angular 1.5.5 and angular-material 1.0.7. Here is a demo with no ng-if's or ui-router(I want to use it with ui-router but the error doesn't require it). http://codepen.io/anon/pen/EKBdBy?editors=101

jraadt commented 8 years ago

@epelc I just pushed up some fixes. It should work better for you now. I pulled down your codepen html and js into my local project and tested against that. It works now and I used Angular 1.4.8 and Material 1.0.7.

I noticed with your codepen the sticky header gets a few pixels off for the widths of the cells. I haven't seen that in the real project I'm using it with, so I'll have to do some more debugging to see why it's not calculating the cell widths correctly in your codepen. That part I think is copied exactly from @daniel-nagy's code.

Also, I modified your codepen to get it to work. Something in your CSS was not working. http://codepen.io/anon/pen/eZwbzy

rafaelhc commented 8 years ago

@jraadt Great solution to this problem! From your codepen, I can not identify exactly what you have changed. I am currently using daniel-nagy/fixed-table-header, what will I have to change to get your version to work?

rafaelhc commented 8 years ago

I have included all your changes, and cannot get it to work. What I can see is that the md-sticky-clone table is not getting the values sticky-state="active" sticky-prev-state="next.

Might it be the $mdSticky not running correctly?

Running on angular version 1.5.5, anybody experiencing the same?

jraadt commented 8 years ago

I'm running on Angular 1.4.8 and Material 1.0.8. Sounds like your $mdSticky isn't activating. Can you create a codepen?

rafaelhc commented 8 years ago

I cannot seem to host the versions that I am using, therefore not being able to create a proper copy.

However, I started a blank angular project, added angular,, angular-aria, angular-animate, angular-material, angular-material-data-table and angular-fixed-table-header (your version). Added the code from your codepen.

Versions:

`"angular": "~1.5.0"`,
`"angular-material-data-table": "~0.10.9",`
`"angular-fixed-table-header": "*"`

Even tried copying all your sources files (rawgit and googleapis etc)

Still not working

rafaelhc commented 8 years ago

@jraadt I have implemented your version inside md-tabs, I am suspecting this is what might be causing the issue. Forking your codepen, and putting the table inside one of the tabs, will not trigger $mdSticky: http://codepen.io/rafaelcode/pen/JKyyRP?editors=1010

Any ideas?

jraadt commented 7 years ago

@rafaelhc The entire page is scrolling. My solution works best when the md-content (inside the md-tab) is scrolling and the tabs and toolbar are fixed to the top. That's how I use it.

rafaelhc commented 7 years ago

@jraadt Thanks, that works fine. However, I see two problems with your solution:

  1. If the table needs horizontal scrolling, the sticky table header will not follow on scroll.
  2. Does not work for touch devices
jraadt commented 7 years ago

@rafaelhc

  1. It works with horizontal scrolling for me. You need to ensure the outside container that both the table and the fixed header are children of (md-content I believe) has the scroll. The fixed header is absolutely positioned to that so it should scroll.
  2. I haven't tested on touch devices.
rafaelhc commented 7 years ago

@jraadt Horizontal scrolling is not working in your prototype. See this image

When scrolling the table horizontally, the table header stays fixed. In the image, you can see that Carbs (g) for example is placed on top of the comment column etc.

The md-content gets an overflow-x, causing it to be wider than the screen width. overflowx

jraadt commented 7 years ago

You want md-content to have the overflow-x: auto so the entire thing, including fixed header, can scroll. You don't want that inner scroll so set the md-table-container to overflow-x: visible.

By no means am I trying to indicate that my solution is best/right solution. I'm hoping that it will lead to a better, more universal solution so I wanted to throw it out there for others to start hacking on. It works in most of my current use cases, but I'd welcome any updates to make it work better universally.

rafaelhc commented 7 years ago

@jraadt Thanks, that works pretty close to what I`m after. I appreciate your patience:)

mikila85 commented 7 years ago

Somone going to merge this?

AnkurChoraywal commented 7 years ago

Please merge it !

viktarkorshun commented 7 years ago

+1 for merge

leonardochaia commented 7 years ago

I'd love to see a solution without depending on Angular Material.

egantz commented 7 years ago

any progress with the merge? would help us all! thanks! 👍

abhishekpabba commented 7 years ago

@jraadt I have implemented your version inside md-virtual-repeat-container and I am getting the following two errors 1) Cannot read property 'insertBefore' of undefined 2) Controller 'mdVirtualRepeatContainer', required by directive 'mdVirtualRepeat', can't be found! Below is the codepen URL for the same issue. I'd love to see a solution which can work with md-virtual-repeat-container. http://codepen.io/abhishekpabba/pen/ygVLwV image

jrcollins4 commented 7 years ago

I would love to see this merged! Thanks for all the great work, @jraadt.

benn0r commented 7 years ago

I would love to use this too, but so far i have not seen a working codepen with the source from @jraadt. I copied the source code from https://github.com/jraadt/fixed-table-header/blob/master/src/fixed-table-header.js into the original codepen (http://codepen.io/anon/pen/zZroBJ?editors=1010) but its not working (table header doesn't stick to the top). I am testing with Google Chrome and Safari. Can someone help?

nithyaswam commented 7 years ago

Hi I also tried this workaround as suggested by @jraadt but the table header doesnt stick to the top. Can someone provide a codepen with a lot of rows to show that the table header is sticking to the top with @jraadt changes. May be I am missing something.

mckinleymedia commented 6 years ago

@abhishekpabba. I've got the same issue. Did you solve this?

hetal0713 commented 6 years ago

Anyone able to resolve this issue? we don't have ng-if on any of the table element still get this error when using fix-head attribute.

"angular.js:14982 Error: [$compile:ctreq] Controller 'mdTable', required by directive 'mdColumn', can't be found!"