-
Notifications
You must be signed in to change notification settings - Fork 414
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
Escape link for tags #2984
Escape link for tags #2984
Conversation
Thanks for the pr!! I'll give this a closer look soon but it looks good. |
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.
One small change. Can you also add an addition to this file?
https://github.com/lbryio/lbry-desktop/blob/master/CHANGELOG.md
src/ui/component/tag/view.jsx
Outdated
@@ -15,7 +15,7 @@ type Props = { | |||
export default function Tag(props: Props) { | |||
const { name, onClick, type = 'link', disabled = false } = props; | |||
const isMature = MATURE_TAGS.includes(name); | |||
const clickProps = onClick ? { onClick } : { navigate: `/$/tags?t=${name}` }; | |||
const clickProps = onClick ? { onClick } : { navigate: encodeURIComponent(`/$/tags?t=${name}`) }; |
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.
This should only wrap the name
variable so the routing still works. Can you change this to
{ navigate: `/$/tags?t=${encodeURIComponent(name)}` };
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.
Oof sorry about that, I've fixed it. Also updated the changelog
Merged! Thanks again for the PR! |
Thanks for the guidance and being patient! |
@ksami , thanks for the PR! Can we show you some appreciation for the contribution? Also, we're giving away some Hacktoberfest bonuses and goodies for this month. We'll get it all figured out after you shoot us an email after this is reviewed/merged. |
PR Checklist
Please check all that apply to this PR using "x":
PR Type
What kind of change does this PR introduce?
Fixes
Issue Number: #2678
Escapes the generated link for tags by using
encodeURIComponent