Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions src/Controls/src/Core/Shell/ShellNavigationManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,12 @@ public void HandleNavigated(ShellNavigatedEventArgs args)
_shell.PropertyChanged += WaitForWindowToSet;
var shellContent = _shell?.CurrentItem?.CurrentItem?.CurrentItem;

if (shellContent != null)
shellContent.ChildAdded += WaitForWindowToSet;
shellContent?.ChildAdded += WaitForWindowToSet;

_waitingForWindow = new ActionDisposable(() =>
{
_shell.PropertyChanged -= WaitForWindowToSet;
if (shellContent != null)
shellContent.ChildAdded -= WaitForWindowToSet;
shellContent?.ChildAdded -= WaitForWindowToSet;
});

void WaitForWindowToSet(object sender, EventArgs e)
Expand Down Expand Up @@ -320,7 +318,7 @@ public static void ApplyQueryAttributes(Element element, ShellRouteParameters qu
var mergedData = MergeData(element, filteredQuery, isPopping);

//if we are pop or navigating back, we need to apply the query attributes to the ShellContent
if (isPopping && mergedData.Count > 0 )
if (isPopping && mergedData.Count > 0)
{
element.SetValue(ShellContent.QueryAttributesProperty, mergedData);
}
Expand All @@ -341,6 +339,21 @@ public static void ApplyQueryAttributes(Element element, ShellRouteParameters qu
element.SetValue(ShellContent.QueryAttributesProperty, mergedData);
}
}
else if (element is Page)
{
// Intermediate page (not the last item, not wrapped in ShellContent).
// Apply prefix-filtered query parameters directly via the attached property,
// which triggers OnQueryAttributesPropertyChanged to handle IQueryAttributable,
// BindingContext propagation, and [QueryProperty] attributes.
if (filteredQuery.Count == 0 && !element.IsSet(ShellContent.QueryAttributesProperty))
return;

var mergedData = MergeData(element, filteredQuery, isPopping);
if (mergedData.Count > 0 || !isPopping)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 95e0ab7. Added the ShellContent-style guard:

if (filteredQuery.Count == 0 && !element.IsSet(ShellContent.QueryAttributesProperty))
    return;

This skips empty queries on fresh intermediate pages. For pages that previously had params set (re-navigation), an empty query still propagates to allow clearing.

Added test IntermediatePageWithNoPrefixedParamsDoesNotReceiveEmptyQuery to verify.

{
element.SetValue(ShellContent.QueryAttributesProperty, mergedData);
Comment on lines +351 to +354
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — but this is a pre-existing issue in the ShellRouteParameters(query, prefix) constructor (line 55-56 of ShellRouteParameters.cs). It copies _shellNavigationQueryParameters verbatim for ALL prefix-filtered queries, including those used in the ShellContent branch at line 311. The intermediate page branch inherits the same behavior.

Fixing this would require changing the shared constructor, which would also affect the ShellContent path. I think that warrants a separate issue/PR rather than mixing it into this bug fix.

}
}

ShellRouteParameters MergeData(Element shellElement, ShellRouteParameters data, bool isPopping)
{
Expand Down
202 changes: 202 additions & 0 deletions src/Controls/tests/Core.UnitTests/ShellParameterPassingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,208 @@ public async Task ShellSectionChangedSetsQueryAttributesProperty()
"QueryAttributesProperty should be set when changing ShellSection");
}

[Fact]
Comment thread
mattleibow marked this conversation as resolved.
public async Task IntermediatePageReceivesPrefixedQueryParamsViaIQueryAttributable()
{
var shell = new TestShell();
var item = CreateShellItem(shellSectionRoute: "section", shellContentRoute: "content");
shell.Items.Add(item);

var intermediatePage = new ShellTestPage();
shell.RegisterPage("product", intermediatePage);
Routing.RegisterRoute("review", typeof(ShellTestPage));

await shell.GoToAsync("product/review?product.sku=seed-tomato&stars=5");

// Intermediate page should have received the prefixed param "sku"
Assert.Single(intermediatePage.AppliedQueryAttributes);
Assert.True(intermediatePage.AppliedQueryAttributes[0].ContainsKey("sku"));
Assert.Equal("seed-tomato", intermediatePage.AppliedQueryAttributes[0]["sku"]);

// Last page should have received "stars"
var lastPage = shell.CurrentPage as ShellTestPage;
Assert.NotNull(lastPage);
Assert.NotEqual(intermediatePage, lastPage);
Assert.Single(lastPage.AppliedQueryAttributes);
Assert.True(lastPage.AppliedQueryAttributes[0].ContainsKey("stars"));
Assert.Equal("5", lastPage.AppliedQueryAttributes[0]["stars"]);
}

[Fact]
public async Task IntermediatePageReceivesQueryPropertyAttributes()
{
var shell = new TestShell();
var item = CreateShellItem(shellSectionRoute: "section", shellContentRoute: "content");
shell.Items.Add(item);

var intermediatePage = new ShellTestPage();
shell.RegisterPage("product", intermediatePage);
Routing.RegisterRoute("review", typeof(ShellTestPage));

await shell.GoToAsync($"product/review?product.{nameof(ShellTestPage.SomeQueryParameter)}=hello");

Assert.Equal("hello", intermediatePage.SomeQueryParameter);
}

[Fact]
public async Task LastPageStillReceivesUnprefixedParamsWithIntermediatePages()
{
var shell = new TestShell();
var item = CreateShellItem(shellSectionRoute: "section", shellContentRoute: "content");
shell.Items.Add(item);

Routing.RegisterRoute("product", typeof(ShellTestPage));
Routing.RegisterRoute("review", typeof(ShellTestPage));

await shell.GoToAsync($"product/review?{nameof(ShellTestPage.SomeQueryParameter)}=world&stars=5");

// Last page (review) should get the unprefixed params
var lastPage = shell.CurrentPage as ShellTestPage;
Assert.NotNull(lastPage);
Assert.True(lastPage.AppliedQueryAttributes[0].ContainsKey(nameof(ShellTestPage.SomeQueryParameter)));
Assert.Equal("world", lastPage.AppliedQueryAttributes[0][nameof(ShellTestPage.SomeQueryParameter)]);
Assert.True(lastPage.AppliedQueryAttributes[0].ContainsKey("stars"));
}

[Fact]
public async Task MultipleIntermediatePagesEachReceiveOwnPrefixedParams()
{
var shell = new TestShell();
var item = CreateShellItem(shellSectionRoute: "section", shellContentRoute: "content");
shell.Items.Add(item);

var categoryPage = new ShellTestPage();
var productPage = new ShellTestPage();
shell.RegisterPage("category", categoryPage);
shell.RegisterPage("product", productPage);
Routing.RegisterRoute("review", typeof(ShellTestPage));

await shell.GoToAsync("category/product/review?category.name=seeds&product.sku=tomato&stars=5");

// First intermediate page gets category-prefixed params
Assert.Single(categoryPage.AppliedQueryAttributes);
Assert.True(categoryPage.AppliedQueryAttributes[0].ContainsKey("name"));
Assert.Equal("seeds", categoryPage.AppliedQueryAttributes[0]["name"]);

// Second intermediate page gets product-prefixed params
Assert.Single(productPage.AppliedQueryAttributes);
Assert.True(productPage.AppliedQueryAttributes[0].ContainsKey("sku"));
Assert.Equal("tomato", productPage.AppliedQueryAttributes[0]["sku"]);

// Last page gets unprefixed params
var lastPage = shell.CurrentPage as ShellTestPage;
Assert.NotNull(lastPage);
Assert.True(lastPage.AppliedQueryAttributes[0].ContainsKey("stars"));
Assert.Equal("5", lastPage.AppliedQueryAttributes[0]["stars"]);
}

[Fact]
public async Task OverlappingParamNamesDeliverCorrectValueToEachPage()
{
var shell = new TestShell();
var item = CreateShellItem(shellSectionRoute: "section", shellContentRoute: "content");
shell.Items.Add(item);

var intermediatePage = new ShellTestPage();
shell.RegisterPage("product", intermediatePage);
Routing.RegisterRoute("review", typeof(ShellTestPage));

// Both pages have "name" as key, but intermediate gets it via prefix
await shell.GoToAsync("product/review?product.name=IntermediateValue&name=DetailValue");

// Intermediate page should get "name=IntermediateValue"
Assert.Single(intermediatePage.AppliedQueryAttributes);
Assert.True(intermediatePage.AppliedQueryAttributes[0].ContainsKey("name"));
Assert.Equal("IntermediateValue", intermediatePage.AppliedQueryAttributes[0]["name"]);

// Last page should get "name=DetailValue", NOT "IntermediateValue"
var lastPage = shell.CurrentPage as ShellTestPage;
Assert.NotNull(lastPage);
Assert.NotEqual(intermediatePage, lastPage);
Assert.True(lastPage.AppliedQueryAttributes[0].ContainsKey("name"));
Assert.Equal("DetailValue", lastPage.AppliedQueryAttributes[0]["name"]);
}

[Fact]
public async Task IntermediatePageDoesNotReceiveUnprefixedParams()
{
var shell = new TestShell();
var item = CreateShellItem(shellSectionRoute: "section", shellContentRoute: "content");
shell.Items.Add(item);

var intermediatePage = new ShellTestPage();
shell.RegisterPage("product", intermediatePage);
Routing.RegisterRoute("review", typeof(ShellTestPage));

// "stars" is unprefixed — should only go to the last page
// "product.sku" is prefixed — should only go to intermediate
await shell.GoToAsync("product/review?product.sku=seed-tomato&stars=5&name=Alice");

// Intermediate page should only get "sku" (from "product.sku")
Assert.Single(intermediatePage.AppliedQueryAttributes);
Assert.True(intermediatePage.AppliedQueryAttributes[0].ContainsKey("sku"));
Assert.Equal("seed-tomato", intermediatePage.AppliedQueryAttributes[0]["sku"]);
// Should NOT contain unprefixed params
Assert.False(intermediatePage.AppliedQueryAttributes[0].ContainsKey("stars"));
Assert.False(intermediatePage.AppliedQueryAttributes[0].ContainsKey("name"));

// Last page should get both unprefixed params
var lastPage = shell.CurrentPage as ShellTestPage;
Assert.NotNull(lastPage);
Assert.True(lastPage.AppliedQueryAttributes[0].ContainsKey("stars"));
Assert.True(lastPage.AppliedQueryAttributes[0].ContainsKey("name"));
Assert.Equal("Alice", lastPage.AppliedQueryAttributes[0]["name"]);
}

[Fact]
public async Task IntermediatePageWithNoPrefixedParamsDoesNotReceiveEmptyQuery()
{
var shell = new TestShell();
var item = CreateShellItem(shellSectionRoute: "section", shellContentRoute: "content");
shell.Items.Add(item);

var intermediatePage = new ShellTestPage();
shell.RegisterPage("product", intermediatePage);
Routing.RegisterRoute("review", typeof(ShellTestPage));

// No prefixed params for "product" — only unprefixed params for last page
await shell.GoToAsync("product/review?name=Alice");

// Intermediate page should NOT receive ApplyQueryAttributes at all
Assert.Empty(intermediatePage.AppliedQueryAttributes);

// Last page should still get the param
var lastPage = shell.CurrentPage as ShellTestPage;
Assert.NotNull(lastPage);
Assert.Single(lastPage.AppliedQueryAttributes);
Assert.Equal("Alice", lastPage.AppliedQueryAttributes[0]["name"]);
}

[Fact]
public async Task IntermediatePageReceivesParamsOnReNavigation()
{
var shell = new TestShell();
var item = CreateShellItem(shellSectionRoute: "section", shellContentRoute: "content");
shell.Items.Add(item);

var intermediatePage = new ShellTestPage();
shell.RegisterPage("product", intermediatePage);
Routing.RegisterRoute("review", typeof(ShellTestPage));

// First navigation: push product + review
await shell.GoToAsync("product/review?product.sku=tomato&stars=5");
Assert.Single(intermediatePage.AppliedQueryAttributes);
Assert.Equal("tomato", intermediatePage.AppliedQueryAttributes[0]["sku"]);

// Navigate back to root
await shell.GoToAsync("//section/content");

// Second navigation with different params
await shell.GoToAsync("product/review?product.sku=pepper&stars=3");
Assert.Equal(2, intermediatePage.AppliedQueryAttributes.Count);
Assert.Equal("pepper", intermediatePage.AppliedQueryAttributes[1]["sku"]);
}

// ApplyQueryAttributes should NOT be called on an inactive (non-target) page's ViewModel
// when navigating back with query params on the second cycle through the same pages.
[Fact]
Expand Down
Loading
Loading