feat: add Breadcrumbs Mode.ROUTER navigation listener and trail builder#9510
feat: add Breadcrumbs Mode.ROUTER navigation listener and trail builder#9510web-padawan wants to merge 2 commits into
Conversation
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
web-padawan
force-pushed
the
feat/breadcrumbs-task7
branch
from
6b70eef to
39e5b4c
Compare
The navigationRegistration field was transient, so the reference was lost on session serialization while the AfterNavigationListener stayed registered on the (serializable) UI. A later detach could then not remove the listener, leaking it. Registration is Serializable and every other component stores it non-transient; drop transient to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
There was a problem hiding this comment.
I may have missed this in a previous review: IMO this should not extend from AbstractSignalsTest, and should instead declare the required MockUIExtension directly.
| .getInstantiator(); | ||
| return RouteUtil | ||
| .resolvePageTitle(instantiator, reference.navigationTarget(), | ||
| reference.routeParameters(), QueryParameters.empty()) |
There was a problem hiding this comment.
Query parameters might be relevant for building the page title? A view might as well use products?product=5 instead of /products/5, if just for legacy reasons. For example, an existing app is updated to use DynamicPageTitle but the URL pattern that is used today must not be changed.
What I'm not sure about is whether query parameters should be passed to ancestor items in the trail or not. My hunch is that passing them only to the last item is a better default. Whatever the decision is, it should be mentioned in the JavaDoc.
2 participants
Description
Fixes #9481
Task 5 of the Breadcrumbs SDD.
Added
Mode.ROUTERbuilding logic and navigation listener.Type of change