pjax
pjax copied to clipboard
MoOx
Resize / scroll triggered multiple times after a page transition
I have a page with multiple elements whose content pjax controls and each of those might use a different "switch" to swap the old content out for the new. After every page transition I have a bunch of scripts that handle how the new content is supposed to behave. Some of these need to also update on 'resize'.
The problem is that pjax triggers 'resize' and 'scroll' on every element "switch", so in my case would trigger resize & scroll 6 times for every page transition. The method responsible for that is: https://github.com/MoOx/pjax/blob/master/index.js#L72
My solution was to override the aforementioned onSwitch method with the following:
let switched = true;
Pjax.prototype.onSwitch = function onSwitch() {
if (!switched) return;
switched = false;
// 'once' is implemented to fire event cb once and then de-attach
once(window, 'pjax:complete', () => {
Pjax.trigger(window, 'resize scroll');
switched = true;
});
};
This seems to work so far, but I am wondering whether there is a better way and/or whether pjax should handle triggering resize & scroll only once after a page transition internally.
@MoOx i think the bigger question here is whether switches should trigger window resize/scroll at all.
Why not:
- expose switch event with pjax:switch. Users should be responsible for triggering resize if they need it.
- users can then decide if they want to debounce multiple pjax:switch events or not. (as OP)
- scroll should only fire once, if the scrollTo position is being changed.
As far as I can tell, resize/scroll events is not even necessary for basic usage. However, I haven't yet used this library thoroughly. Maybe you will remember something.
@darylteo if you click at a link in the bottom of the page and do not make an automatic scroll, the user experience is pretty weird: the page content change, but you are in the middle of the new content... Not sure what is the best. FYI I don't use this library anymore, so feel free to push some idea/PRs ;)
Hmm... I think based on my experience, most of the time you'd expect to scroll to the top of the container while the content is loading. Devs can also choose to clear the target container and replace it with a loading indicator. There's also no guarantee that they want smooth scroll - and might prefer to jump to scroll position instead.
I feel letting the developer decide what is best UX for their application is better rather than enforcing UX.
I know you don't use this library anymore, but I don't know how you'd feel regarding changes to the library and the concepts and principles it subscribes to. Thoughts?
I feel letting the developer decide what is best UX for their application is better rather than enforcing UX.
Fair enough :) You can probably get rid of that in the code. Just add something in the doc on "how to handle scroll" that will work for 90% of the users.
@MoOx What's the effect of triggering the resize and scroll events? Is it only to notify the user, or does it have an effect on the browser as well?
I agree with @darylteo that we should deprecate triggering resize and scroll.
I also think that we don't need to trigger a new event like pjax:switch when using a user-created switch. Since onSwitch() is called immediately after each switch, if the user wants to do anything afterwards, they can just append that to their custom switch.
Instead, I think it should trigger pjax:switch at the end of the built-in switches, to allow the user to take some action at that point.
Regarding scrolling, I think we should do as follows:
- When loading a new page, scroll to
options.scrollTo. - When the new URL has a hash (e.g.
example.com#main), attempt to find that on the page, and if found, scroll to it. (See #22). - When navigating backwards and forwards, store the scroll position when pushing state, and then scroll back to that position when popping state. (See #30).
What about always triggering pjax:switch event but send a flag to say if it's user created or built-in? (for consistency)
Scrolling proposal LGTM.
If it's triggered by every switch, that would mean doing it in onSwitch(), but there's no way at that point to tell which switch triggered it.
I think there are 3 options when to trigger pjax:switch:
- In
onSwitch, which will trigger on every switch, and let the user debounce the multiple events (as @darylteo proposed earlier). - In
afterAllSwitches, which will trigger only once after all the switches finish. - At the end of each of the default switches, which would mean it does not get triggered automatically by user-created switches.
I think either triggering an event during each onSwitch or once in afterAllSwitches seem most useful -- if doing it in onSwitch, we could pass the switch selector that was responsible for the switch as an event payload, so that users can choose to either debounce the multiple events or respond to each one individually (e.g. respond to a synchronous switch as soon as it's done and then waiting to respond to an async switch later after a transition/animation).
I already do something similar in my current project that's using Pjax whereby I have a custom async switch to handle switching the main page content which dispatches pjax:switch:before and pjax:switch:after events to allow me to respond to that particular switch independently of anything responding to pjax:complete.
Why do you need events? Why can't you do whatever you need right inside the custom switch?
@BehindTheMath I use events because my JS is modularised to ensure separation of concerns -- I have a component that is responsible for setting up Pjax and related functionality (topbar loading indicator etc.) and defining my custom switch, but other components that add specific functionality to my main content (e.g. a masonry grid) need to know when a switch is happening so that they can destroy and/or init themselves at the right time.
Got it.
if doing it in onSwitch, we could pass the switch selector that was responsible for the switch as an event payload
This would only work if the user passed the element to onSwitch(), which is currently not done. I guess we could add a note that if you pass it, it will be sent along with the event.
so that users can choose to either debounce the multiple events
This will be even easier if we switch to an event emitter (see #53).
This would only work if the user passed the element to onSwitch(), which is currently not done. I guess we could add a note that if you pass it, it will be sent along with the event.
Good point -- we could do that, or we could pass a pre-bound callback to each switch function to call at the end of the switch process.
i.e in lib/switches-selectors.js, do something like var onSwitch = this.onSwitch.bind(this, selector) and pass it in to each switch.
This will be even easier if we switch to an event emitter
Sounds like a good idea -- but maybe something to do after pushing out a new release? Such a change would probably be a fairly extensive change and necessitate a major version bump (because it would be breaking compatibility with previous versions). Adding more user-facing events is already a minor version bump, according to SemVer.
Good point -- we could do that, or we could pass a pre-bound callback to each switch function to call at the end of the switch process.
i.e in lib/switches-selectors.js, do something like var onSwitch = this.onSwitch.bind(this, selector) and pass it in to each switch.
That's also a breaking change. I think it will be simpler to just have the user pass the element to onSwitch.
Sounds like a good idea -- but maybe something to do after pushing out a new release? Such a change would probably be a fairly extensive change and necessitate a major version bump (because it would be breaking compatibility with previous versions). Adding more user-facing events is already a minor version bump, according to SemVer.
We have a breaking change already, I moved the trigger for the resize and scroll from onSwitch to afterAllSwitches without thinking about this issue. I could move it back, but switching from those events to pjax:switch is regardless going to be a breaking change.
But I think you're right. I'll move the trigger back. We can finish up with any other PRs that won't have breaking changes, release 0.2.5, then milestone this and everything else for 0.3.0.
@MoOx Do you mind if we use milestones to keep track of what we want to implement when?