Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Added a check for a route fragment in the navigationEnd event #232

Conversation

Blackbaud-BrandonJones
Copy link
Contributor

The navigationEnd subscription was causing pages to scroll to the top of the page on every navigation event. This was breaking in-page navigation to fragments on the page. With this change, it allows apps to scroll to elements on their page based on the fragment, as is expected from in page anchor links.

@codecov-io
Copy link

codecov-io commented Jul 19, 2017

Codecov Report

Merging #232 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #232   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          54     54           
  Lines        1221   1223    +2     
  Branches      177    178    +1     
=====================================
+ Hits         1221   1223    +2
Flag Coverage Δ
#builder 100% <ø> (ø) ⬆️
#runtime 100% <ø> (ø) ⬆️
#srcapp 100% <100%> (ø) ⬆️
Impacted Files Coverage Δ
src/app/app.component.ts 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0378241...cc67411. Read the comment docs.

@Blackbaud-SteveBrush Blackbaud-SteveBrush self-assigned this Jul 20, 2017
@@ -54,7 +54,12 @@ describe('AppComponent', () => {
events: {
subscribe: handler => subscribeHandler = handler
},
navigateByUrl: url => navigateByUrlParams = url
navigateByUrl: url => navigateByUrlParams = url,
parseUrl: url => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check your spaces count after the lambda, and after the undefined, below.

@Blackbaud-SteveBrush
Copy link
Member

@Blackbaud-PaulCrowder I don't see anything wrong with this one. You?

@Blackbaud-PaulCrowder
Copy link
Member

Seems fine to me.

@Blackbaud-SteveBrush Blackbaud-SteveBrush merged commit 8a49f46 into blackbaud:master Jul 25, 2017
Blackbaud-DiHuynh pushed a commit to Blackbaud-DiHuynh/skyux-builder that referenced this pull request Jul 10, 2020
Bumps [minimist](https://github.com/substack/minimist) from 1.2.0 to 1.2.2.
- [Release notes](https://github.com/substack/minimist/releases)
- [Commits](https://github.com/substack/minimist/compare/1.2.0...1.2.2)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants