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

scrollContainer, boundary, display change issues #58

Closed
chriswks opened this issue Nov 8, 2019 · 3 comments
Closed

scrollContainer, boundary, display change issues #58

chriswks opened this issue Nov 8, 2019 · 3 comments
Labels
question Further information is requested. Awaiting response. review Issue probably fixed. Awaiting response.

Comments

@chriswks
Copy link

chriswks commented Nov 8, 2019

  1. ScrollContainer doesn't seem to be respected, to overcome it I input in an array of elements to the parent component that sums their offsetHeight's and then sets the marginTop of the directive. Which works well enough and allows to recalc the heights of the elements above it on resize.

  2. ScrollContainer + Boundary doesn't invoke the fixed positioning at all (in the previous v1.1.9 when both were specified it would still invoke the fixed position but boundary was not respected and the sticky element wouldn't detach on boundary end).

  3. A reset input that could fire on an property change to recalc the position etc would be really handy for scenarios when say the parent toggles display none.

On a side note, I'm trying to get permission to contribute back during work hours or find some spare time to go fork and submit some PR's back to ya and not try and mooch but you should consider adding a donate button since obviously this repo is getting traction and can assume more will come as it gets refined, cheers! 💯 👍

@ghost
Copy link

ghost commented Nov 11, 2019

Hello @chriswks, thanks for the input. We've just released v.1.2.3 where the scroll container issue is (almost) fixed, but note that until now boundary doesn't work inside scroll container. Also updated the README with a new example and a screencast for further information. Hope this already helps. Of course some pull requests would be also highly appreciated. 🤓

@ghost ghost added review Issue probably fixed. Awaiting response. question Further information is requested. Awaiting response. labels Nov 11, 2019
@ghost ghost added this to Backlog in sticky-things-issues Nov 11, 2019
@ghost ghost moved this from Backlog to In progress in sticky-things-issues Nov 11, 2019
@ghost ghost moved this from In progress to Review in sticky-things-issues Nov 11, 2019
@chriswks
Copy link
Author

Awesome! I'll try grab the latest npm soon (it's not registering with an install and only ref'd of up to 1.2.2 at the moment so I'll just assume npm has to catch up or something). I'll try and carve out some time to go put in towards stuff like keeping a boundary container within a scroll-able element. Which I had started to look at but quickly found that even on a debounce doing an element.getBoundingClientRect().top becomes an eye sore on the 'ol performance tree. However I'd really like to use this thing in a greater capacity so will try to throw more time at it from home before I leave for vacation. Assuming you're Deutsch ich schulde dir ein bier wenn ich das nächste mal dort bin...either way 🤣

@ghost
Copy link

ghost commented Nov 12, 2019

Stimmt genau. 🍻 I'll close it for now and wait for specific pull requests. Thanks in advance with best regards.

@ghost ghost closed this as completed Nov 12, 2019
sticky-things-issues automation moved this from Review to Done Nov 12, 2019
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested. Awaiting response. review Issue probably fixed. Awaiting response.
Projects
Development

No branches or pull requests

1 participant