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

use window scroll position for ember-router-scroll #579

Merged
merged 1 commit into from
Jul 16, 2018

Conversation

bgentry
Copy link
Contributor

@bgentry bgentry commented Jul 13, 2018

When using the addon docs, I was getting annoyed that the documentation app did not scroll-to-top when clicking to a different docs section. This fixes that.

In the ember-cli-addon-docs quickstart, they mention ember-router-scroll as an optional dependency to tackle this problem.

This installs the ember-router-scroll addon to properly remember scroll position between routes, and to reset it when the route changes. There was already a (copy-pasted?) config for this in the dummy app, but it pointed to an element that doesn't exist. Instead I just removed the config and it now uses the window scroll position instead, which works well.

@bgentry
Copy link
Contributor Author

bgentry commented Jul 13, 2018

I'm noticing now that ember-cli-addon-docs actually includes ember-router-scroll as a direct dependency now. Perhaps its docs are out of date and we don't need to install it directly, only to make sure that it is configured? Gonna try that out to see if it was really just a problem due to bad config.

This configures the ember-router-scroll addon to properly remember
scroll position between routes. This is installed as part of
ember-cli-addon-docs, but its config in the dummy app here pointed to an
element that doesn't exist. Instead I just removed the config and it now
uses the `window` scroll position instead, which works well.
@bgentry bgentry force-pushed the ember-router-scroll-in-docs branch from 3555204 to 416f2f3 Compare July 13, 2018 18:44
@bgentry bgentry changed the title install ember-router-scroll, use window scroll position use window scroll position for ember-router-scroll Jul 13, 2018
@bgentry
Copy link
Contributor Author

bgentry commented Jul 13, 2018

I updated the PR to merely fix the config for ember-router-scroll, which appears to work. Also opened a corresponding issue to fix the docs at ember-learn/ember-cli-addon-docs#203.

@pzuraq pzuraq merged commit 6c3cf7f into Addepar:master Jul 16, 2018
@pzuraq
Copy link
Contributor

pzuraq commented Jul 16, 2018

Thanks for the fix! Was wondering why that was happening

@bgentry bgentry deleted the ember-router-scroll-in-docs branch July 16, 2018 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants