Enable backbutton for same routes

[bcardarella]

Closes #12
Closes #11

• Moved mixin to root of project so the README is accurate (Consider moving mixin location #11)
• Added new Location object that will add unique IDs to the History
  API's current state. This allows for the scroll position to be
  preserved across multiple instances of the same route in the history.
  (Scroll position for historical pages #12)
• Added a service to store the scroll information

This change is breaking the previous API as it now requires that the
Location object be added to config/environment.js. To enable this:

// config/environment.js
locationType: 'router-scroll'

[bcardarella]

I believe that Ember's HistoryLocation should be adding a unique id for each history state by default. I'm going to make a PR to Ember to enable this. If/when that gets pulled into a future version of Ember this addon can drop the custom location.

[bcardarella]

For reference: emberjs/ember.js#14011

[bcardarella]

I added a service. The reason is to deal with multiple app instances. With the key/scrollMap being stored in the module this created a global value that could collide if you have multiple apps consuming. For example, in the test suite if parallel tests are running.

[briangonzalez]

This looks solid, @bcardarella! We'll review internally and merge later on today. Thank you 🙏

[bcardarella]

Awesome. I'm looking to add some unit tests, however unit testing against window is tricky. Might have something by EOD

[bennycwong]

@bcardarella Thanks for your work on this. I'm trying to run it now in our application, but it doesn't seem to be hitting the router-scroll extension of HistoryLocation.

I validated that when running in chrome, the pushState(path) and replaceState(path) of add-on/locations/router-scroll.js never executes.

I've added locationType: "router-scroll" to config/environment.js. However, I'm not sure if this allows you to override the default. I looked at the Ember Guides and they state that there are only 4 options: history hash auto none
https://guides.emberjs.com/v2.0.0/configuring-ember/specifying-url-type/

In the meantime, I'm going to merge in #13 so the guides make sense for users.

[bcardarella]

@bennycwong the guides are "correct" in that Ember only ships with four, however it uses the owner API to lookup a given location: location:<name> so as long as there is a module being exported at app/locations/<name>.js it should find it.

[briangonzalez]

So, @bcardarella, it should look like this inside folks' apps?

// app/locations/router-scroll.js
export { default } from 'ember-router-scroll/locations/location.js';

A la ember-cli-sentry

[bcardarella]

@briangonzalez no, this PR already should add the location to the app path. There is a app/locations/router-scroll.js file. Unless there is something happening with your Broccoli setup to disallow app paths in addons.

I have this branch running on dockyard.com without issue. I could probably do a screen hero or a hangout in about 45 minutes if that would be helpful

[bcardarella]

Just updated with a rabase against master

[bennycwong]

@bcardarella Ah! I see where I went wrong. I needed to update the app/router.js as well:

//app/router.js

Router.reopen({
  location: 'router-scroll'
});

[bcardarella]

did you have it explicitly set to a value? It should be pulling from your config by default:

https://github.com/ember-cli/ember-cli/blob/master/blueprints/app/files/app/router.js#L5

[bennycwong]

@bcardarella It turns we originally had Router.reopen force 'history', which is why I had to override it in the above comment. This looks good. We can merge it in.

We really appreciate your help here. Keep a lookout for us at ElixirConf this year.

[bennycwong]

Looks like at some point during the rebase, the add-on/index.js changes got removed.

import Ember from 'ember';

const {
  get,
  inject,
  run: { next },
} = Ember;

export default Ember.Mixin.create({
  service: inject.service('router-scroll'),

  willTransition(...args) {
    this._super(...args);
    get(this, 'service').update();
  },

  didTransition(transitions, ...args) {
    this._super(transitions, ...args);
    next(() => {
      let scrollPosition = get(this, 'service.position');

      let preserveScrollPosition = transitions[transitions.length - 1]
        .handler.controller.get('preserveScrollPosition');

      if (!preserveScrollPosition) {
        window.scrollTo(scrollPosition.x, scrollPosition.y);
      }
    });
  }
});

#15

I'll go a head and clean this up.

[bcardarella]

👍

I see that you tagged v0.0.5 but it hasn't been pushed to npm?

[briangonzalez]

@bennycwong @bcardarella

Done.

[bcardarella]

awesome :)