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

Update README.md to correct missing manager method: addStickyHeaderToTable #4

Open
ghost opened this issue Oct 13, 2016 · 4 comments
Open
Assignees
Labels

Comments

@ghost
Copy link

ghost commented Oct 13, 2016

Looks like the manager only has add now?

https://github.com/bsara/sticky-table-headers.js/blob/master/README.md#manual-initialization

Upon further inspection of this library -- I was hoping to use the document body as my scrollableElement. Not having any luck with that so far. If I have any luck and time I'll update, but it may be worth noting in the readme if that isn't possible.

@bsara
Copy link
Owner

bsara commented Oct 13, 2016

@kcaron-cst what is preventing you from using the document body as the scrollableElement? I'll admit that the docs need some attention here...but I'm using this library in the way that you've described (assuming I'm understanding you correctly) with no issues. What can I do to help?

@ghost
Copy link
Author

ghost commented Oct 13, 2016

@bsara Thank you for the quick reply! I think I must just be doing something dumb but I'm not sure where yet. Here is a JS Bin that gives as simple a scenario as I can. I'm loading 0.2.1 (not the auto init version). I'm expecting that when the window is short enough to have scrollbars, that when I scroll past the table headers, that they stick to the top.

I'm using the add method here instead of init as my real use case involves multiple tables and I need to control when the elements are in the DOM. Appreciate any help you can provide.

http://jsbin.com/wetuzapera/edit?html,js,output

@bsara
Copy link
Owner

bsara commented Oct 14, 2016

OK, @kcaron-cst, I've found you're problem. It's a good one too. 👍

Here's how things work in the library:

  • A listener is set up to listen for any scrolling of scrollableElement.
  • Once scrollableElement scrolls, the position of the thead element of your table.sth-sticky element is updated via the style attribute and a tiny CSS class.

However, there is one problem, if the element you declare as your scrollableElement is not actually the element being scrolled, then the scroll event will never be called for scrollableElement. That's exactly what is happening here. Though it may look like the body element is being scrolled, unless you specifically set body to overflow: auto; and html to overflow: hidden;, then the window object is what scrolls, not the body element. You can see evidence of this if you omit the scrollableElement parameter from the function call in your JS Bin example. (Also, you are referencing the auto init version of the script in your JS Bin, not the manual version, but that wasn't a big deal).

So, to fix your problem (until I update the library to accept the window object as a valid scrollableElement), you should add the following to your CSS:

html {
  overflow: hidden;
}

body {
  overflow: auto;
}

If that is present, you can use STH.manager.add or STH.enableStickyTableHeader without issue when passing body as the scrollableElement parameter.

I've updated your example to a working state in the following JS Bin: http://jsbin.com/giyiyoc/1/edit?css,js,console,output

I will mark this issue as an open bug so that I remember to fix the window issues.

Let me know if you have any questions or if I can help with anything else.

P.S. I plan on refactoring this library eventually to just make it a custom HTML element that extends table, which could make things work much more cleanly in the future. In retrospect, I see that the manager and other aspects of the library's implementation are actually kind of confusing, and I hope to at least simplify things in the future.

@bsara bsara added the bug label Oct 14, 2016
@bsara bsara self-assigned this Oct 14, 2016
@bsara
Copy link
Owner

bsara commented Oct 14, 2016

Also, @kcaron-cst feel free to hit me up on Gitter if you need more help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant