Blazor 6.0 query string enhancements#23000
Merged
Merged
Conversation
Updates Updates
41 tasks
MackinnonBuck
requested changes
Aug 12, 2021
Member
MackinnonBuck
left a comment
There was a problem hiding this comment.
Looks awesome! Just a small suggestion relating to the removal some functionality from GetUriWithQueryParameter. The tables are a great idea - I agree they are a useful way to illustrate the exact behavior of these APIs. I also agree that it's fine to use %20 everywhere.
MackinnonBuck
approved these changes
Aug 12, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #22689
Addresses #22045
Internal Review Topic (links directly to the NavManager table section; the Query strings section immediately follows)
Mackinnon ...
I'd normally 🛌😴 one more night, but let's get this in quickly 🏃. It will be reviewed again at RC.
Most of this PR pertains to the query string bits, but I do add the
replaceguidance forNavigateTowhile I'm here.URI vs. URL: The API rightly uses "URI" ... but for practical purposes around the doc set, we use URL because that's specifically and usually what the dev is working with. Here, "URI" is used in text near API remarks, and "URL" is generally used elsewhere.
+and%20in encoding. Let's NOT get deep into the weeds on it! 😄 lol ... There's a delta in the blog post remarks/examples. I think we want%20everywhere.WRT the example tables for URL mods: Don't sweat what looks like awfully wide table cells at this point. Yes! ... they are scary 😨 W....I....D....E for rendering 😄. I want to see how they render in the live topic, then I'll make mods to the display on a follow-up PR as needed. I gave your test code names the Rex treatment 😆 ... movie stars from Serenity.
Can't stop the signal, Mack. Everything goes somewhere, and I go everywhere. - Mr. Universe (David Krumholtz)
Quote ©2005 Universal Pictures: Serenity https://www.uphe.com/movies/serenity David Krumholtz on IMDB: https://www.imdb.com/name/nm0472710/