Skip to content

fix(ng-lib): getCurrent method needs decodeURI#560

Merged
SanderElias merged 1 commit intoscullyio:masterfrom
stevermeister:getCurrent-bug
May 18, 2020
Merged

fix(ng-lib): getCurrent method needs decodeURI#560
SanderElias merged 1 commit intoscullyio:masterfrom
stevermeister:getCurrent-bug

Conversation

@stevermeister
Copy link
Copy Markdown
Contributor

@stevermeister stevermeister commented May 17, 2020

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Other... Please describe:

What is the current behavior?

Condition

curLocation === r.route.trim()

does not work because curLocation is not decoded, so there are some extra symbols there.

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@jorgeucano jorgeucano requested a review from SanderElias May 18, 2020 10:59
@SanderElias
Copy link
Copy Markdown
Contributor

@stevermeister What problem does this solve?
Can you provide some context?

@stevermeister
Copy link
Copy Markdown
Contributor Author

stevermeister commented May 18, 2020

@SanderElias yes, sure here you are:

https://scullyio.netlify.app/
if you click on the first post - you see the title
on the second - you are getting error in the console

and here is the code -
https://github.com/stevermeister/blog/blob/master/src/app/blog/blog.component.ts#L24

it does not work because URL in the browser is - .../blog/2018/10/dutch-tax-income-calculator%253A-now-with-offline-mode, but it tried to compare with .../blog/2018/10/dutch-tax-income-calculator%3A-now-with-offline-mode (without %25)

@SanderElias
Copy link
Copy Markdown
Contributor

Good catch

@SanderElias SanderElias merged commit d39967e into scullyio:master May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants