-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Includes query parameters in rendered Link component #2464
Conversation
Visit the preview URL for this PR (updated for commit 8c17b85): https://yew-rs-api--pr2464-fix-yew-router-link-14wtmzvq.web.app (expires Mon, 28 Feb 2022 15:07:57 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
fbd2d0f
to
8336b32
Compare
8336b32
to
8c17b85
Compare
Could you someone start review this PR? |
Hi @hamza1311 @futursolo, could you start a review of this PR? If there is a missing point, please let me know. |
I believe most maintainers are busy at the moment and they will eventually get to this pull request. (BTW, your pull request is fine.) |
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.
Sorry for the wait. Nice work and looks good to me 💯
@voidpumpkin Thanks! No problem, I know you're busy. Should I wait response from @hamza1311 or ask other maintainers? (PR requires 2 reviewers) @futursolo Thanks! But I misunderstood that you're a maintainer of Yew because I saw your name many times in this repo... |
@kuy Currently mostly only me and @hamza1311 review Pr's. @siku2 sometimes hops in to help but not that often. |
Thanks @hamza1311 😄 |
Description
<Link>
component ofyew-router
works well even if it has query parameters, but it doesn't render query parameters inhref
attribute of<a>
tag (DOM tree). This PR resolves this issue by fixing<Link>
component.Let me explain by using
router
example. Here is a snippet from<Pagination>
component.Assuming
to_page
variable has2
value, the above code will be rendered...Expected:
<a href="/posts?page=2">2</a>
Actual:
<a href="/posts">2</a>
Again, this is just an issue on DOM representation, not behavior.
Checklist
cargo make pr-flow