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

Router event when navigating back in history #436

Closed
powerbuoy opened this issue Nov 25, 2016 · 21 comments
Closed

Router event when navigating back in history #436

powerbuoy opened this issue Nov 25, 2016 · 21 comments

Comments

@powerbuoy
Copy link

I'm trying to determine whether the new route was processed because the user clicked the back button (or router.navigateBack() or similar was called) or if it was a "normal" click.

I've studied the router object itself, as well as the argument passed to the EventAggregator callback on the router:navigation:processing event but there doesn't seem to be any property indicating whether the back button was used or the route was initiated normally.

The reason I'd like to know this is so that I can animate my page transitions differently depending on whether the user is backing or going forward (they all sort of go forward now which looks odd when you actually hit the back button).

I've tried to store the history myself but it just feels wrong. Surely there's a better way to know than to re-create the entire browser history locally?

@EisenbergEffect
Copy link
Contributor

There's no API that enables us to know whether the back button was clicked. We would have to come up with a mechanism of determining "direction" within that router that was independent of the back button. If you have some thoughts on how we can enhance the router to enable that, we'd love to do it.

@powerbuoy
Copy link
Author

@EisenbergEffect Thanks Rob, I love how active you are with the community. Unfortunately I feel I don't know anywhere near enough about the internals of Aurelia to suggest anything meaningful regarding this. I was hoping the router somehow knew the difference between going back in history as opposed to going to a new route/adding to the history.

Do you have any idea how this can be solved right now, without any changes to the router? Even if I did store the entire history in a local array, I still wouldn't be able to tell whether the user clicked the back button in his browser to reach the previous page, or whether he actually clicked a link or button somewhere that also navigated him to the previous page.

We might just use a different, more "direction neutral" animation, for page transitions if this proves too difficult to solve.

@EisenbergEffect
Copy link
Contributor

@bryanrsmith We talked about this issue a little while back I think. Did you have some thoughts on how we might accomplish it?

@powerbuoy
Copy link
Author

I'm not sure this is related at all, but I've noticed that when I click back, or use router.navigateBack() the scroll position is restored to where it was on the previous page.

I enter page A, scroll down 800px, enter page B, I'm still on 800px scroll position, I scroll up to 0px, click back, the browser scrolls down to the 800px it was at page A and then navigates back to page A.

If there's code in place to restore the scroll position on back navigation, perhaps an event could be popped around the same time?

Also, it is a little annoying that navigating back does restore the scroll position, but when navigating forward it does NOT scroll to top. Is this intentional?

@alu
Copy link

alu commented Dec 14, 2016

I had same issue for scroll control. My workaround is below.

import {PLATFORM} from 'aurelia-pal';

export default class {
  configureRouter(config, router) {
    config.options.pushState = true;

    let onPopstate = false;
    PLATFORM.addEventListener('popstate', _ => {
      // popstate event will happen on back or forward operation
      onPopstate = true;
    });

    config.addPreRenderStep({run: (navigationInstruction, next) => {
      if (!onPopstate) {
        PLATFORM.global.scrollTo(0, 0);
      }

      onPopstate = false;
      next();
    }});
  }
}

@powerbuoy
Copy link
Author

@alu Hmm ok, thanks, but I'm not sure I follow. Where exactly do you know it was a backwards navigation? Could you add a comment like // Add code here that only runs on backwards nav or similar? Is it on the !onPopState if or?

@alu
Copy link

alu commented Dec 19, 2016

@powerbuoy popstate events are happen on back/forwards both.I find how to catch just back event, i couldn't found it, sorry.

@jwx
Copy link
Member

jwx commented Jan 13, 2017

@EisenbergEffect I have a solution for the following state flags:

isNavigatingFirst: true when navigating into the app for the first time in browser session
isNavigatingNew: true when navigating to an instance not in browser session history
isNavigatingBack: true when navigating back in browser session history
isNavigatingForward: true when navigating forward in browser session history
isNavigatingRefresh: true when "navigation" is a browser refresh

Router events could be fired based on this when appropriate.

It does however utilize history.state which might need a polyfill so it's not entirely clean. Is this interesting?

@EisenbergEffect
Copy link
Contributor

Yes. Absolutely. Can you take some time to put together a bit more detailed info on how you'll accomplish this. I'd like to get a few others to look at it in addition to me such as @jdanyow and @bryanrsmith

@jwx
Copy link
Member

jwx commented Jan 13, 2017

@EisenbergEffect If you're okay with the usage of history.state and the possible polyfill I could put together a PR. I'll just make sure those mentioned flags are correct and any eventual events can be added later. The code will be pretty self explanatory (and really quite simple).

(If you don't like the history.state usage, I'll probably be able to do a couple of the flags, but not all of them. It'll also be a lot more complicated.)

@jwx
Copy link
Member

jwx commented Jan 13, 2017

@EisenbergEffect Forgot: Shall I create the PR?

@EisenbergEffect
Copy link
Contributor

Yes, go ahead. Let's see what it looks like and we can go from there.

@jwx
Copy link
Member

jwx commented Jan 14, 2017

@EisenbergEffect Sorry to ask a basic question, but I haven't managed to find the information through google or the gitter channel. The question:

This PR will involve more than one Aurelia repo -- router and browser-history (and perhaps even a new one called storage) -- and I'm wondering how you do a branch/PR in such a way that you keep consistency between the repos? (I'm assuming there's a better way, both in git and when actually working with the changes, than just using the same branch name on the different repos.)

@EisenbergEffect
Copy link
Contributor

We don't really do anything fancy. You an just send the PR and then in the PR description indicate that it's dependent on the other PR.

@jwx
Copy link
Member

jwx commented Jan 14, 2017

@EisenbergEffect Oh, that was easy. I can do that. :) I have another (better) question:
The BrowserHistory will store and retrieve data from the history.state. I've got an "old, faithful servant" called Storage that "implements a unified store and retrieve storage for local and remote stores" which I would normally use for this (it's also the one polyfilling history.state functions if necessary). This is more a general purpose module rather than something that's specific to the router or history, so what's your prefered way when it comes to structure? Should I create a separate (trimmed down) aurelia/storage module that browser-history uses or should I extract the relevant code into browser-history?

@jwx
Copy link
Member

jwx commented Jan 14, 2017

Here's the POC: http://aurelia-navigation-flags.azurewebsites.net/

@powerbuoy
Copy link
Author

Just wanted to say that I think this looks super promising :)

@EisenbergEffect
Copy link
Contributor

@jwx You could create an "abstract class" that the history module depends on and then provide a simplified implementation that will work for standard use cases. You could define the interface for this class to match your more advanced functionality so that then a simple DI configuration could allow it to be swapped out. Does that make sense?

@jwx
Copy link
Member

jwx commented Jan 15, 2017

@EisenbergEffect Yeah. On the other hand, this module doesn't really have any advanced functionality. Especially if I take the remote stores out (which won't be necessary here). To make it easy, I think I'll just start with removing everything not dealing with the ordinary local storages.

jwx added a commit to jwx/history-browser that referenced this issue Jan 16, 2017
…state

Set and get a key's value for the current page in the browser history.

Depending on new module aurelia-storage (currently in jwx/storage).
Necessary for PR router/navigation-flags (aurelia/router#436).
jwx added a commit to jwx/router that referenced this issue Jan 16, 2017
The added properties allows deciding on behaviour during the routing lifecycle based on user navigation choices.

Depending on PR aurelia/history-browser/page-state.
Closes aurelia#436.
jwx added a commit to jwx/router that referenced this issue Jan 16, 2017
The added properties allows deciding on behaviour during the routing lifecycle based on user navigation choices.

Depending on PR aurelia/history-browser/page-state.
Closes aurelia#436.
@jwx
Copy link
Member

jwx commented Jan 16, 2017

@powerbuoy Glad to hear it. The PRs are in, hopefully it'll deliver. :)

jwx added a commit to jwx/history-browser that referenced this issue Feb 2, 2017
…state

Set and get a key's value for the current page in the browser history.

Depending on new module aurelia-storage (currently in jwx/storage).
Necessary for PR router/navigation-flags (aurelia/router#436).
jwx added a commit to jwx/router that referenced this issue Feb 2, 2017
The added properties allows deciding on behaviour during the routing lifecycle based on user navigation choices.

Depending on PR aurelia/history-browser/page-state.
Closes aurelia#436.
jwx added a commit to jwx/router that referenced this issue Feb 2, 2017
The added properties allows deciding on behaviour during the routing lifecycle based on user navigation choices.

Depending on PR aurelia/history-browser/page-state.
Closes aurelia#436.
jwx added a commit to jwx/router that referenced this issue Feb 3, 2017
The added properties allows deciding on behaviour during the routing lifecycle based on user navigation choices.

Depending on PR aurelia/history-browser/page-state.
Closes aurelia#436.
jwx added a commit to jwx/history-browser that referenced this issue Apr 6, 2017
…state

Set and get a key's value for the current page in the browser history.

Necessary for PR router/navigation-flags (aurelia/router#436).
@meash-nrel
Copy link

This PR going to merge? sounds promising

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

No branches or pull requests

5 participants