search

nk-o / jarallax

Parallax scrolling for modern browsers
https://jarallax.nkdev.info
MIT License
1.39k stars 214 forks source link

Wrap parallax on element #85 #155

Closed omiroshnichenko closed 4 years ago

omiroshnichenko commented 4 years ago

Fixes #85

davemacaulay commented 4 years ago

@omiroshnichenko did you run their associated tests and ensure we have test coverage where we need it?

omiroshnichenko commented 4 years ago

@omiroshnichenko did you run their associated tests and ensure we have test coverage where we need it?

Yes

davemacaulay commented 4 years ago

@omiroshnichenko we should also update the TypeScript definition to include the new option: https://github.com/nk-o/jarallax/blob/master/typings/index.d.ts

omiroshnichenko commented 4 years ago

@omiroshnichenko we should also update the TypeScript definition to include the new option: https://github.com/nk-o/jarallax/blob/master/typings/index.d.ts

Done.

nk-o commented 4 years ago

Hey.

Thank you for contribution. I have checked your code a little and I have some questions:

  1. Why do you limit it for a single oldElementData? Users may set option and will be able to add unlimited different containers on one page and your code will not work with it.
  2. I see you added function updateElementParallax, which is called every tick, which is ok, but you call another function getParentBySelector, it will go over all parent elements every single tick. I see you already checked it on the initialization step, so you will not need to check it again.
  3. Why do you not use updateParallax, but create a copy of it? jarallaxList is important part, which contains all parallax blocks.
  4. What about more complex situations like deep scrollable blocks? For example:
    • Scrollable Window
      • Scrollable Div
        • Another Scrollable Div
        • Jarallax

Maybe we can make something better, that can detect scrollable parents automatically? Something like this does Popper.js plugin. Example:

omiroshnichenko commented 4 years ago

Hey.

Thank you for contribution. I have checked your code a little and I have some questions:

  1. Why do you limit it for a single oldElementData? Users may set option and will be able to add unlimited different containers on one page and your code will not work with it.
  2. I see you added function updateElementParallax, which is called every tick, which is ok, but you call another function getParentBySelector, it will go over all parent elements every single tick. I see you already checked it on the initialization step, so you will not need to check it again.
  3. Why do you not use updateParallax, but create a copy of it? jarallaxList is important part, which contains all parallax blocks.

  4. What about more complex situations like deep scrollable blocks? For example:

    • Scrollable Window

      • Scrollable Div

      • Another Scrollable Div

        • Jarallax

Maybe we can make something better, that can detect scrollable parents automatically? Something like this does Popper.js plugin. Example:

  1. containerList has been introduced to store all containers that include jarallax instances in container and scroll position
  2. Removed
  3. updateParallax was updated.
  4. Currently, we don't have such a case. It could be done in the scope of other PR/improvement task.

Auto detecting of a scrollable parent has been implemented.