-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
fix(progress-bar): incorrectly handling current path when using hash location strategy #12713
Conversation
src/lib/progress-bar/progress-bar.ts
Outdated
this._rectangleFillValue = `url('${location ? location.path() : ''}#${this.progressbarId}')`; | ||
// because named route URLs can contain parentheses (see #12338). Also we don't use | ||
// `Location` from `@angular/common`, because we can't tell the difference between whether | ||
// the consumer is using the hash location strategy or not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't you do the same thing with location.path()
from @angular/common
's Location
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what it used to do, but AFAIK Angular's location.path
will normalize both of these to the same value: /#/foo/bar
and /foo/bar
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, got it. Can you expand the comment with that?
...the consumer is using the hash location strategy or not because it normalizes both
`/#/foo/bar` and `/foo/bar` to the same thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
…location strategy Fixes the progress bar prefixing the references incorrectly when the consumer is using the router's hash location strategy. The issue comes from the fact that the router normalizes the `location.pathname` between the regular strategy and the hash one, whereas we need to use the exact same path as the `window.location`. Fixes angular#12710.
62b7288
to
a3f5206
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…location strategy (#12713) Fixes the progress bar prefixing the references incorrectly when the consumer is using the router's hash location strategy. The issue comes from the fact that the router normalizes the `location.pathname` between the regular strategy and the hash one, whereas we need to use the exact same path as the `window.location`. Fixes #12710.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes the progress bar prefixing the references incorrectly when the consumer is using the router's hash location strategy. The issue comes from the fact that the
Location
normalizes thelocation.pathname
between the regular strategy and the hash one, whereas we need to use the exact same path as thewindow.location
.Fixes #12710.