afterFirstRoutePaint instead of afterContentPaint

[snewcomer]

This change simply puts the scrollTop functionality one frame before it currently runs (which afterContentPaint is two frames after running scheduleWork in didTransition).

I currently see jitter as the DOM elements of the next route are showing before scrollTop happens. Visually, this reduces the time the next route content is seen before scrollTop happens; however, perhaps more work can be done to tighten this loop.

Also, I might be missing some context, but what is the reason we want to scrollTop after the content has painted? I'm currently seeing that without the scheduleWork functionality, visually the app is better b/w transitions w/ and w/o fetch/xhrs...

Ref #57 #17

[snewcomer]

Also note the seeing the next route elements before scrollTop is especially apparent for routes that will transition without making a fetch/xhr request (perhaps if your Ember data model is already loaded).

[RobbieTheWagner]

I don't know enough about this, but seems logical. @briangonzalez would you like to weigh in?

[snewcomer]

I just to note, here is the README from ember-app-scheduler. If we want to include the scrollTop functionality with the first paint of the route without any jumping/jank, then it may be important to reduce the delayed time it takes to scrollTop.

Ember batches DOM updates and paints them after every run loop to prevent layout thrashing. This prevents a faster First Meaningful Paint (FMP) because all the content of the page is painted at once. For a large scale application though, some work done on the page like ads, analytics tracking, rendering non critical content, rendering content outside viewport etc. can be deferred to achieve a faster First Meaningful Paint (FMP). This work can be scheduled to run after the First Meaningful Paint and achieve incremental rendering of the page.

[RobbieTheWagner]

@snewcomer are there any potential downsides to this change? I don't know much about this myself, but it seems like it could be fine. I just don't want to change the current behavior or introduce anything that jumps or appears glitchy.

[snewcomer]

@rwwagner90 This PR just moves one tick before to here. I guess this PR wasn't necessarily to merge this, but to perhaps figure out an improved pattern. Even in the demo app, it's really easy to see the jumping (well not easy easy 😆 ). Removing ember-app-scheduler all but eliminates this jumping when transitioning b/w routes.

Based on #57, the basic premise was to add ember-app-scheduler to delay the scrollTop a few ticks and referenced #17. However, I believe #17 actually is saying the "delay" was the problem and wanted to tighten the loop, not lengthen it. Moreover, #57 (at the comments below) only seeks out to solve the case when the route is rendered in chunks, not if the model hooks pauses for all the information and then paints (which it worked well for before).

In our app, w/o ember-app-scheduler, there is no glitchyness and no jump after transition. With it's current form there is. With this PR, there is less.

@cclo7 Do you happen to have any thoughts here as well?

[RobbieTheWagner]

@snewcomer so should we remove ember-app-scheduler completely then? I haven't done much in this codebase, so I'm not familiar with the internals.

[snewcomer]

@rwwagner90 I think it would be good to see if anybody familiar with some history could chime in; however, given this problem is visible in the demo app, it's prob something we should solve.

One option would be to set a configuration variable. immediateScrollTop: true for now and this simply updates the scroll position on First Meaningful Paint, rather than deferring it with ember-app-scheduler. I'll put a quick proof of concept for this if you agree.

[snewcomer]

Or delayScrollTop: true and make the default immediate. Which I think is the better option.