Improve 'GetUriWithQueryParameter' examples#37158
Open
guardrex wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Blazor navigation documentation to make the GetUriWithQueryParameter(s) guidance easier to follow by clarifying the Navigation instance used in examples and showing fully functional navigation calls.
Changes:
- Added a NOTE that
Navigationrefers to an injectedNavigationManager, with an@injectsnippet. - Updated
GetUriWithQueryParameter(s)examples to pass the generated URI toNavigation.NavigateTo(...)so the examples perform navigation. - Reformatted several examples as shorter C# call snippets.
Comments suppressed due to low confidence (2)
aspnetcore/blazor/fundamentals/navigation.md:994
- The text under “For the preceding example” describes a returned string (“A string is returned…”), but the preceding code sample now calls
NavigateTo(...)(which doesn’t return a URI). Either adjust the prose to refer specifically to the innerGetUriWithQueryParametercall, or revise the sample to capture the returned URI before callingNavigateTo.
```csharp
Navigation.NavigateTo(Navigation.GetUriWithQueryParameter("{NAME}", {VALUE}));
For the preceding example:
- The
{NAME}placeholder specifies the query parameter name. The{VALUE}placeholder specifies the value as a supported type. Supported types are listed later in this section. - A string is returned equal to the current URL with a single parameter:
- Added if the query parameter name doesn't exist in the current URL.
- Updated to the value provided if the query parameter exists in the current URL.
- Removed if the type of the provided value is nullable and the value is
null.
**aspnetcore/blazor/fundamentals/navigation.md:1014**
* Same as above: the bullet list repeats the `{PARAMETERS}` placeholder type as `IReadOnlyDictionary<string, object>`, which conflicts with the `object?` usage in the examples (null values). Update this to `IReadOnlyDictionary<string, object?>` for consistency.
- The
{URI}placeholder is the URI with or without a query string. - The
{PARAMETERS}placeholder is anIReadOnlyDictionary<string, object>.
</details>
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 #37157
Wade, Tom ... Just need one review to get this in.
Noticed this while working with Daria on the QuickGrid PR, where we needed to use
GetUriWithQueryParameterin a QuickGrid example.I'd like to do two things here to improve this batch of examples in the Navigation article ...
GetUriWithQueryParameterexamples thatNavigationin the examples is an injectedNavigationManager. After that NOTE, all of the examples can just be C# calls with a shorter code display.NavigateTocalls to theGetUriWithQueryParameterexamples to make them fully functional calls.GetUriWithQueryParameterby itself just gets a URI.WRT Copilot's recommendation to state
Dictionary<string, object?>for the remark aboutGetUriWithQueryParameterscalls, that's true.IReadOnlyDictionaryisn't the best API to specify because (IIRC) the method signature is forIReadOnlyDictionary... that's why it was there. If I had focused the sentences on something about the method signature, that probably would've been fine. However, Copilot is correct ... it should indicate what the developer is passing.Internal previews