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

HistoryLocation Subclass Question #193

Closed
stefanpenner opened this issue Apr 18, 2019 · 1 comment · Fixed by #206
Closed

HistoryLocation Subclass Question #193

stefanpenner opened this issue Apr 18, 2019 · 1 comment · Fixed by #206

Comments

@stefanpenner
Copy link

I was reading the code for addon/locations/router-scroll.js and I'm not totally sure if it is required any longer.

The two methods it overwrites are pushState and popState, but when reading ember's latest history_location implementation, it appears to essentially do the same. The exception being, the subclasses methods, do not set this._historyState.

If the reason for the subclass is to avoid setting _historyState then, i suppose it needs to remain. (Although a comment as to why, would be great). But if we don't need to avoid setting _historyState then we should likely just use the default history location object.

@snewcomer
Copy link
Collaborator

snewcomer commented Jun 11, 2019

Agreed. Can't believe this hasn't been removed! Plan for removal in 2.0

#203

snewcomer added a commit that referenced this issue Jan 4, 2021
snewcomer added a commit that referenced this issue Jan 6, 2021
* chore(deps): upgrade `ember-cli-babel`

This fixes the following error:

```
Build Error (broccoli-persistent-filter:Babel > [Babel: @ember/test-helpers]) in @ember/test-helpers/dom/-is-focusable.js

Cannot find module '@babel/compat-data/corejs3-shipped-proposals'
Require stack:
- /Users/jan/open-source/ember-router-scroll/node_modules/@babel/preset-env/lib/polyfills/corejs3/usage-plugin.js
- /Users/jan/open-source/ember-router-scroll/node_modules/@babel/preset-env/lib/index.js
- /Users/jan/open-source/ember-router-scroll/node_modules/@babel/core/lib/config/files/plugins.js
- /Users/jan/open-source/ember-router-scroll/node_modules/@babel/core/lib/config/files/index.js
- /Users/jan/open-source/ember-router-scroll/node_modules/@babel/core/lib/index.js
- /Users/jan/open-source/ember-router-scroll/node_modules/broccoli-babel-transpiler/lib/worker.js
```

* refactor: first shot at using `RouterService`

* refactor to master

* fix lockfile

* some improvements to tests.  more to go

* fix some tests

* Fix tests

* fix bug in recursive call

* Update docs

* rm deprecation outdated

* Do not need locations/router-scroll

#193

* CHANGELOG 4.0.0

* Update addon/services/router-scroll.js

Co-authored-by: John Leja <[email protected]>

* fix comment

* Address some feedback

* Update addon/services/router-scroll.js

Co-authored-by: Chantal Broeren <[email protected]>

* moar updates

* address moar feedback

Co-authored-by: Jan Buschtöns <[email protected]>
Co-authored-by: John Leja <[email protected]>
Co-authored-by: Chantal Broeren <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants