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

[WIP] Support native sticky #70

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

ctjhoa
Copy link
Collaborator

@ctjhoa ctjhoa commented Oct 7, 2018

Here is a PR to use native position: sticky if browser supports it.
I think it's good to share my work on that and iterate with maintainers to get at a consensus.
This could bring performance improvements as a majority of browsers support sticky
https://caniuse.com/#feat=css-sticky

Here is the current test results on this branch:

1..182
# tests 182
# pass  181
# skip  0
# fail  1

I haven't add new tests yet.

I try to be the most compatible with the existing interface unfortunately there is some limitations:

old
2018-10-08-071030_2464x2064_scrot

new
2018-10-07-182329_2592x1802_scrot

I would like your thoughts on this.

@ctjhoa
Copy link
Collaborator Author

ctjhoa commented Oct 8, 2018

One solution to not break compatibility could be to add this as an opt-in feature.

@simonihmig
Copy link
Member

@ctjhoa first thanks for your effort here, and sorry for the late response!

Before getting into details, can you explain the motivation for this change? I guess it's mainly about performance, right? But as the native template also includes the trigger components (to continue supporting the yielded isSticky* states I guess), which adds the scroll listeners/InteractionObserver from ember-in-viewport, I wonder if we really get any noticeable performance improvement?

@ctjhoa
Copy link
Collaborator Author

ctjhoa commented Oct 28, 2018

@simonihmig You're right it's for performance reasons.

I think that performance can be split in 2 things.

The first thing is the rendering performance which is way better when you let the browser use it's native implementation. Component are rendered smoothly even when you scroll very fast and there is no more "jumpy" rendering cause by javascript computations.

Then there are isSticky* states performance which still rely on ember-in-viewport and listerners/observers. This is not tackle with my PR and I do not know better way to achieve that. Even google complains about the lack of event regarding sticky states.

I find more important for the final users to see smooth rendering even if internal states are updated later. So using position: sticky directly may cause latency between what users saw and the internal state. I do not use isSticky* states so I don't know what drawbacks can occurred.

@spr1ne
Copy link

spr1ne commented Feb 11, 2020

position: fixed always relative to browser window, if i will want start ember application in some container - position: fixed, it will break my app. In this case, maybe it’s worth completing the task? I can help with it. @simonihmig

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

Successfully merging this pull request may close these issues.

3 participants