feat!: match query params when determining highlighted items

[vursen]

Description

The PR adds new logic to side-nav to match query params when determining highlighted items. A side-nav item is highlighted if the browser URL has the same base origin, the same path, and at least contains all the query parameters from the item's path.

Here is a table that illustrates which browser URLs would match which item URLs with the new logic:

Item URL ➡️ Browser URL ⬇️	/products	/products?c=socks	/products?c=	/products?c=socks&c=pants	/products?socks
/products	✅	❌	❌	❌	❌
/products?c=socks	✅	✅	❌	❌	❌
/products?c=pants	✅	❌	❌	❌	❌
/products?c=	✅	❌	✅	❌	❌
/products?c=socks&item=5	✅	✅	❌	❌	❌
/products?item=5&c=socks	✅	✅	❌	❌	❌
/products?c=socks&c=pants	✅	✅	❌	✅	❌
/products?socks	✅	❌	❌	❌	✅
/products?socks=	✅	❌	❌	❌	✅

Warning

Note that it's a breaking change for some configurations. When using an item's URL to set defaults for dynamic params, such as for filtering or pagination, then changing those params would break matching. For example, if you configure an item with a URL like /products?pageSize=10 and then the page size is changed, the item wouldn't match anymore. Could be fixed on the application side by adding a path alias without the dynamic query params, or applying the defaults based on their absence in the URL.

Fixes #6778

Type of change

• Feature

[vursen]

note: web-test-runner adds a unique query parameter with a session ID to the base URI, which causes some tests to fail because they don't expect it. Stubbing the baseURI solves this problem.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[vursen]

I haven't found a trivial way to stub document.location properties since they can't be rewritten, so I only verified this change through manual testing. Any ideas how to test it are welcome.

[vursen]

Snippet for manual testing:

<vaadin-side-nav>
  <span slot="label">Menu</span>

  <vaadin-side-nav-item path="/dev/side-nav.html">
    /dev/side-nav.html
  </vaadin-side-nav-item>

  <vaadin-side-nav-item path="/dev/side-nav.html?view">
    /dev/side-nav.html?view
  </vaadin-side-nav-item>

  <vaadin-side-nav-item path="/dev/side-nav.html?view=users">
    /dev/side-nav.html?view=users
  </vaadin-side-nav-item>

  <vaadin-side-nav-item path="/dev/side-nav.html?view=products">
    /dev/side-nav.html?view=products
  </vaadin-side-nav-item>

  <vaadin-side-nav-item path="/dev/side-nav.html?view=products&product[]=1">
    /dev/side-nav.html?view=products&product[]=1
  </vaadin-side-nav-item>

  <vaadin-side-nav-item path="/dev/side-nav.html?view=products&product[]=1&product[]=2">
    /dev/side-nav.html?view=products&product[]=1&product[]=2
  </vaadin-side-nav-item>
</vaadin-side-nav>

[sissbruecker]

LGTM! Regarding the following comment from the issue:

When using an item's URL to set defaults for dynamic params, such as for filtering or pagination, then changing those params would break matching. For example, if you configure an item with a URL like /products?pageSize=10 and then the page size is changed, the item wouldn't match anymore. Could be fixed on the application side by adding a path alias without the dynamic query params, or applying the defaults based on their absence in the URL.

As this could be a breaking change for some, let's note this down somehow, so that we can add something to the release notes.

[vursen]

note: The browser URL /products?c=socks&c=pants matches the item path /products?c=socks in my implementation, which deviates from @sissbruecker's proposal – see the case that is marked with a question mark in the table. In my opinion, the implemented behavior is more consistent with other cases.

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.4.0.