refactor(devtools): adding route details panel for selected route #59999

sumitarora · 2025-02-18T18:24:42Z

On clicking a route in the router tree, the route details panel is displayed.

The panel shows the details of the selected route, including the path, component, providers, guards etc.
Clicking on a chip will navigate to the corresponding source file in the editor.
Clicking on the path will navigate to the corresponding route in the router tree.

Demo Application Source: https://github.com/sumitarora/angular-routes-demo
Demo Application Deployed: https://sumitarora.github.io/angular-routes-demo/#/

msmallest · 2025-02-18T20:54:46Z

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/router-tree.component.html

+                @if (route.data.pathMatch) {
+                  <tr ng-route-details-row title="Path Match" [data]="route.data.pathMatch"></tr>
+                }
+                @if (route.data.data && route.data.data.length > 0) {


Would it be safe to shorten checks like these with optional chaining (?.)?

dgp1130

Thanks for putting this together! I know I left a lot of comments, but they're mostly pretty minor refactoring suggestions.

/cc @hawkgs in case he has any thoughts on potential UX improvements.

dgp1130 · 2025-02-20T16:55:12Z

devtools/projects/ng-devtools-backend/src/lib/router-tree.ts

@@ -102,3 +109,52 @@ function childRouteName(child: AngularRoute): string {
    return 'no-name-route';
  }
 }
+
+export function getElementRefByName(ref: string, routes: any[], name: string): any | null {


Suggestion: Some doc comments on these functions would be useful to explain the intent behind them as well as provide context on the terminology. I'm not really sure what ref refers to in a routing context.

dgp1130 · 2025-02-20T16:56:59Z

devtools/projects/ng-devtools-backend/src/lib/router-tree.ts

@@ -102,3 +109,52 @@ function childRouteName(child: AngularRoute): string {
    return 'no-name-route';
  }
 }
+
+export function getElementRefByName(ref: string, routes: any[], name: string): any | null {


Consider: I'm assuming the intended type signature is something like:

import type {Route, CanActivate} from '@angular/router'; export function getElementRefByName(ref: string, routes: Route[], name: string): CanActivate;

Would it be possible to actually do that via a type-only import of @angular/router? I know we can't trivially depend on Router in this context, but I feel like a type-only import might be appropriate?

dgp1130 · 2025-02-20T16:57:42Z

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

+  router.navigateByUrl(path);
+};
+
+export const viewSourceFromRouter = (path: string, type: string) => {


Consider: A doc comment explaining the intent here would be helpful, I'm not sure what "view source" means in this context.

dgp1130 · 2025-02-20T16:58:36Z

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

+  );
+  const rootInjector = (forest[0].resolutionPath ?? []).find((i) => i.name === 'Root');
+  const serializedProviderRecords = getSerializedProviderRecords(rootInjector!) ?? [];
+  const routerInstance = serializedProviderRecords.filter(


Consider: .find might be more appropriate than .filter, given that we expect to find a single router.

dgp1130 · 2025-02-20T17:00:02Z

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

+    value = injector.get(provider.token, null, {optional: true});
+  } else if (Array.isArray(serializedProvider.index)) {
+    const providers = serializedProvider.index.map((index) => providerRecords[index]);
+    value = injector.get(providers[0].token, null, {optional: true});


Consider: We can immediately return in these branches (with an else { return undefined; }) rather than assigning a temporary variable.

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.ts

dgp1130 · 2025-02-20T17:50:01Z

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.ts

+      .tag-incactive {
+        display: inline-block;
+        border-radius: 5px;
+        padding: 2px 8px;


A11y: Should padding and/or border-radius be em or rem units? How does this look when the user zooms in with Ctrl +?

I tried zooming and actually I think the padding looks fine all around.

dgp1130 · 2025-02-20T17:51:18Z

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.ts

+          </mat-chip>
+      } @else if (rowType() === 'flag') {
+        <span  class="{{ data() ? 'tag-active' : 'tag-incactive' }}">
+          {{ data()  ? 'Active' : 'Inactive' }}


Nit: "Active" / "Inactive" I'm assuming apply to guards, but this is rowType() === 'flag', and I wouldn't expect that terminology for a basic boolean. Maybe this should be rowType() === 'guard'?

dgp1130 · 2025-02-20T17:52:55Z

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/router-tree.component.scss

+  display: flex;
+  justify-content: space-between;
+  align-items: center;
+  padding: 10px;


A11y: Similar question here, should these padding properties use em or rem? In my experience that usually scales better.

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.ts

hawkgs · 2025-02-21T10:13:31Z

Here is a list with the areas of the UI that need some adjustments/fixes:

There are some issues with the dark theme in the side panel. Some of the text is always black (Note: We can use the new theme mixins).
The font size of the side panel title (Routes Details) can be decreased a bit. I suggest using the same as the one in the Injector Tree tab.
Same about the search input – it would be good if it matches the one in Injector Tree > Providers for Platform (side panel), at least the font size.
I agree with Doug that using pills for the path and component table values might not be very suitable. Maybe using a monospaced font should be enough.

Overall, I think these are the main points. Anything else can be improved as part of the general effort for introducing consistency across DevTools.

hawkgs · 2025-02-21T10:33:26Z

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

+  const router = getInjectorInstance(rootInjector!, routerInstance[0]);
+  return router;


Can we cast router to Router here? I guess we can benefit from some typing because I noticed that there are some any-s in the diff that can be avoided.

hawkgs · 2025-02-21T10:34:58Z

devtools/projects/ng-devtools-backend/src/lib/router-tree.ts

@@ -102,3 +109,52 @@ function childRouteName(child: AngularRoute): string {
    return 'no-name-route';
  }
 }
+
+export function getElementRefByName(ref: string, routes: any[], name: string): any | null {


routes: any[] => routes: Routes ?

hawkgs · 2025-02-21T12:25:28Z

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.ts

+
+export type RowType = 'text' | 'chip' | 'flag' | 'list';
+
+@Component({


Don't want to steal Doug's conversation about whether this should be a component or a simple template hence the new comment.

Can we create separate files for the template and the styles, in case this remains as a component? I get that this comes up with some additional Bazel build configuration but it should promote better flexibility. Right now, we won't be able to import SCSS files, for instance. This is probably rather a bigger discussion, but in my opinion, we should try to limit the usage of this pattern unless the component has a one-liner template with a single to no styles.

cc @dgp1130 @AleksanderBodurri

dgp1130 · 2025-02-24T23:05:40Z

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

    (provider) => provider.token === 'Router', // get the instance of router using token
  );
-  const routerProvider = getProviderValue(injector, routerInstance[0]);
+  const routerProvider = getProviderValue(injector, routerInstance!);


Bug: I was using DevTools today (actually unrelated to this PR) and noticed errors in the console because router was not on the page and getProviderValue was being called without a router instance.

I feel like the ! assertion is wrong and we should return undefined or otherwise abort if no router could be found?

dgp1130

Some UI feedback:

When there are too many providers it forces horizontal scrolling of the entire panel. I think we can just have those chips line-wrap and we should be good.
Component : text gets a newline as the column attempts to minimize its width. Typically colons don't have spaces in front of them, so Component: would be more appropriate and avoid the line-wrapping issue.
Blue boxes don't highlight when moused over.
If a single chip is long enough to overflow, the horizontal scrollbar should probably go on the details panel rather than the entire router tree panel.
I think I mentioned this when looking at the code, but "Active: Active" and "Auxiliary: Active" read kind of awkwardly. Should these be true / false instead (or a checkbox kind of UI)?
"Children: 1" doesn't seem super helpful to me given that the graph is able to more visually show that. I wonder if this just more noise than it's worth in the table view?
feature/team lists its Component as feature/team [Lazy] but that route doesn't actually have a component listed. I would expect this not to have a component at all since it's just there to load children. Similar issue with Component: no-name on the root node.
Mouseover of the details table highlights using the same color every other route is set to. I suggest either using a slightly different shade or maybe just removing highlighting on mouse hover given that the table only has two columns and I'm not sure highlighting is that useful for building a visual association of items in a row.
At one point I managed to break mouseover highlighting of the graph nodes and it would react exactly backwards. Mousing off a node would highlight it and mousing over a node would remove the highlighting. Resetting DevTools fixed the issue and I haven't been able to reproduce since. So I suspect there's some bug here but I can't narrow it down any more than that.

Feature idea: The "Auxiliary" row could link to the Components panel for the associated <router-outlet> it's rendering to. It would be cool if we could show some meaningful name for the outlet rather than just "Auxiliary: Active", but I'm not sure such a name exists.

General UX question about the route graph more than the sidebar: Have we looked at the accessibility here for vision deficiencies? Chrome has a feature to test out some common forms. My main takeaways are:

Slightly thicker borders on the graph might help low contrast users.
Deuteranopia (no green) users might struggle to distinguish active and eager loaded routes. I wonder if we can tweak the colors a bit to be more helpful here.
Colorblind users can't distinguish the boxes at all.
- I actually think this is mostly ok since the details panel explains textually whether a route is active or lazy and is a reasonable fallback for such users.
- I think the main challenge here is finding the active route, which is difficult without color. I vaguely recall discussing making the line thicker along the path to the active route? That would probably help colorblind users find it more effectively.
For everyone, I'm wondering if the highlight which applies from mousing over a route should also apply the same highlight to the legend in the corner? I wonder if the change in color might be more observable and help draw an association with the specific kind of route they're looking at.

Since this doesn't have too much to do with the route details panel, we can definitely follow up in a separate PR for this color accessibility stuff.

Bug in the demo app: If navigate to "Feature Team Admin" and then refresh, the navigation panel doesn't work and does not navigate anywhere.

dgp1130 · 2025-02-27T16:50:30Z

devtools/projects/ng-devtools-backend/src/lib/router-tree.ts

 // todo(aleksanderbodurri): type these properly
-type AngularRoute = any;
+type AngularRoute = {


Question: Is this is a copy of Route from @angular/router? Feels like it would be easy for this to fall out of date.

I was thinking we could potentially import type to get the benefits of type checking without actually duplicating the router at runtime. Something like:

import type {Route as AngularRoute} from '@angular/router';

Is there a reason we can't do that in this context?

Just FYI, I did this in my split ng interface prototype and it seemed to work without any complications.

44d8a28#diff-90cdacecab0fda39eb99b06085d47ca5d180c5411542c3cd285a7dc50e896ef9

I definitely think this would be a good improvement.

dgp1130 · 2025-02-27T18:01:58Z

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.ts

+      .tag-incactive {
+        display: inline-block;
+        border-radius: 5px;
+        padding: 2px 8px;


I tried zooming and actually I think the padding looks fine all around.

hawkgs · 2025-02-28T08:08:41Z

@dgp1130, I think that we can handle colors in a separate PR as you already pointed. The described issues with the tree visualization are already present in the injector tree since it uses the same styles. We might have to consider high-contrast themes for people with vision deficiencies.

Ps. I've already started working on the color extraction.

sumitarora · 2025-03-06T23:24:48Z

@hawkgs @dgp1130

Updated PR comments and UI issues
Fixed dark mode issues
Changed chips to buttons for clickable links

dgp1130 · 2025-03-11T03:31:36Z

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

+    (provider) => provider.token === 'Router', // get the instance of router using token
+  );
+
+  if (!routerInstance || !rootInjector) {


Bug: We check for !rootInjector here but only after it's been used in getSerializedProviderRecords. I think we want to move this check up.

dgp1130 · 2025-03-11T03:34:44Z

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

@@ -371,6 +400,25 @@ export interface SerializableComponentTreeNode
  children: SerializableComponentTreeNode[];
 }

+function getRouterInstance() {


Consider: Explicit return types.

https://techhub.social/@develwithoutacause/111842243141199473

dgp1130 · 2025-03-11T03:38:21Z

devtools/projects/ng-devtools-backend/src/lib/router-tree.ts

 // todo(aleksanderbodurri): type these properly
-type AngularRoute = any;
+type AngularRoute = {


Just FYI, I did this in my split ng interface prototype and it seemed to work without any complications.

44d8a28#diff-90cdacecab0fda39eb99b06085d47ca5d180c5411542c3cd285a7dc50e896ef9

I definitely think this would be a good improvement.

devtools/projects/ng-devtools-backend/src/lib/router-tree.ts

dgp1130 · 2025-03-11T03:42:25Z

devtools/projects/ng-devtools-backend/src/lib/router-tree.ts

+        return result;
+      }
+    }
+  }


Bug: This function returns undefined if it falls through despite | null being in the type. Probably hidden by the any in the return type.

There is already check when executing the element so even if it returns null should be good.

dgp1130 · 2025-03-11T04:01:23Z

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/router-tree-visualizer.ts

@@ -26,6 +26,10 @@ export class RouterTreeVisualizer {
  private root: RouterTreeD3Node | null = null;
  private zoomController: d3.ZoomBehavior<HTMLElement, unknown> | null = null;

+  protected nodeClickListeners: ((pointerEvent: PointerEvent, node: any) => void)[] = [];
+  protected nodeMouseoverListeners: ((pointerEvent: PointerEvent, node: any) => void)[] = [];
+  protected nodeMouseoutListeners: ((pointerEvent: PointerEvent, node: any) => void)[] = [];


Consider: Can we type node as RouterTreeD3Node?

dgp1130 · 2025-03-11T04:01:53Z

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/router-tree.component.html

+          placeholder="Search routes"
+          class="filter-input"
+        />
+        <mat-checkbox (change)="togglePathSettings()">Show Full Path </mat-checkbox>


Typo: Trailing space.

Suggested change

<mat-checkbox (change)="togglePathSettings()">Show Full Path </mat-checkbox>

<mat-checkbox (change)="togglePathSettings()">Show Full Path</mat-checkbox>

dgp1130 · 2025-03-11T04:04:33Z

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/router-tree.component.ts

+  {position: 8, name: 'Oxygen', weight: 15.9994, symbol: 'O'},
+  {position: 9, name: 'Fluorine', weight: 18.9984, symbol: 'F'},
+  {position: 10, name: 'Neon', weight: 20.1797, symbol: 'Ne'},
+];


Looks like some leftover debug data?

dgp1130 · 2025-03-11T04:05:25Z

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/router-tree.component.ts

@@ -29,9 +66,17 @@ export class RouterTreeComponent {
  private routerTreeVisualizer!: RouterTreeVisualizer;
  private showFullPath = false;

+  private readonly _messageBus = inject(MessageBus) as MessageBus<Events>;
+  private readonly _appOperations = inject(ApplicationOperations);


Style: No need for leading underscores on private members, TS provides protection enough here.

dgp1130 · 2025-03-11T04:10:11Z

...ools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.html

+      <div class="chips-container">
+        @for (p of data(); track $index) {
+          <button mat-stroked-button (click)="click.emit(p)">
+            {{ p }}


Style: Is there a more meaningful name we can use than p? Maybe item?

...ools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.scss

josephperrott

LGTM

Reviewed-for: dev-infra

dgp1130

Thanks for adding the tests! A couple minor comments, but nothing too concerning. If we can address the remaining open comments, I'm hopeful we can get this in pretty easily.

...s/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.spec.ts

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.ts

devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/router-tree.component.html

sumitarora · 2025-04-06T00:50:42Z

@dgp1130 Fixed all comments and pushed everything.

AleksanderBodurri · 2025-04-14T04:21:41Z

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

@@ -166,6 +172,29 @@ const checkForAngularCallback = (messageBus: MessageBus<Events>) => () =>
  checkForAngular(messageBus);
 const getRoutesCallback = (messageBus: MessageBus<Events>) => () => getRoutes(messageBus);

+const navigateRouteCallback = (messageBus: MessageBus<Events>) => (path: string) => {
+  const router: any = getRouterInstance();
+  router.navigateByUrl(path);


We should check for navigateByUrl before calling it here

AleksanderBodurri · 2025-04-14T04:23:50Z

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

+    initializeOrGetDirectiveForestHooks().getIndexedDirectiveForest(),
+    ngDebugDependencyInjectionApiIsSupported(),
+  );
+  const rootInjector = (forest[0].resolutionPath ?? []).find((i) => i.name === 'Root');


I wonder what we should do here for apps with multiple roots 🤔 is it even possible for more than one root to have a router configured, given they'd have to share the url bar? Maybe we should search all roots for the router

cc @dgp1130

alan-agius4 · 2025-04-30T15:19:44Z

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

+    initializeOrGetDirectiveForestHooks().getIndexedDirectiveForest(),
+    ngDebugDependencyInjectionApiIsSupported(),
+  );
+  const rootInjector = (forest[0].resolutionPath ?? []).find((i) => i.name === 'Root');


Suggested change

const rootInjector = (forest[0].resolutionPath ?? []).find((i) => i.name === 'Root');

const rootInjector = forest[0].resolutionPath?.find((i) => i.name === 'Root');

alan-agius4 · 2025-04-30T15:20:31Z

devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

+  const serializedProviderRecords = getSerializedProviderRecords(rootInjector) ?? [];
+  const routerInstance = serializedProviderRecords.find(


Suggested change

const serializedProviderRecords = getSerializedProviderRecords(rootInjector) ?? [];

const routerInstance = serializedProviderRecords.find(

const serializedProviderRecords = getSerializedProviderRecords(rootInjector);

const routerInstance = serializedProviderRecords?.find(

On clicking a route in the router tree, the route details panel is displayed. - The panel shows the details of the selected route, including the path, component, providers, guards etc. - Clicking on a chip will navigate to the corresponding source file in the editor. - Clicking on the path will navigate to the corresponding route in the router tree. Demo Application Source: https://github.com/sumitarora/angular-routes-demo Demo Application Deployed: https://sumitarora.github.io/angular-routes-demo/#/

- Update code to fix PR comments and cleanup code - Add unit tests for the new code

pullapprove bot requested a review from AleksanderBodurri February 18, 2025 18:24

angular-robot bot added the area: devtools label Feb 18, 2025

ngbot bot added this to the Backlog milestone Feb 18, 2025

msmallest reviewed Feb 18, 2025

View reviewed changes

dgp1130 reviewed Feb 20, 2025

View reviewed changes

hawkgs reviewed Feb 21, 2025

View reviewed changes

sumitarora force-pushed the feat-route-details branch 2 times, most recently from 8e7dffe to e3052e8 Compare February 24, 2025 16:11

dgp1130 reviewed Feb 24, 2025

View reviewed changes

dgp1130 reviewed Feb 27, 2025

View reviewed changes

sumitarora force-pushed the feat-route-details branch 5 times, most recently from 7e83df1 to 186f4dd Compare March 6, 2025 20:53

dgp1130 reviewed Mar 11, 2025

View reviewed changes

sumitarora force-pushed the feat-route-details branch 3 times, most recently from 19587c6 to 94064d7 Compare March 17, 2025 15:21

dgp1130 added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Mar 26, 2025

sumitarora force-pushed the feat-route-details branch 4 times, most recently from a1096ad to 6edf626 Compare March 28, 2025 15:52

pullapprove bot requested a review from josephperrott March 28, 2025 15:52

sumitarora force-pushed the feat-route-details branch from 6edf626 to 8705e05 Compare March 28, 2025 16:00

hawkgs reviewed Mar 29, 2025

View reviewed changes

...ools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.scss Outdated Show resolved Hide resolved

josephperrott approved these changes Mar 31, 2025

View reviewed changes

dgp1130 reviewed Apr 2, 2025

View reviewed changes

sumitarora force-pushed the feat-route-details branch 2 times, most recently from 7faa2aa to 4f08705 Compare April 6, 2025 00:25

AleksanderBodurri reviewed Apr 14, 2025

View reviewed changes

sumitarora force-pushed the feat-route-details branch from 4f08705 to 0029445 Compare April 23, 2025 00:20

angular-robot bot requested a review from josephperrott April 23, 2025 00:20

alan-agius4 reviewed Apr 30, 2025

View reviewed changes

sumitarora force-pushed the feat-route-details branch 3 times, most recently from d404c5c to 12373fb Compare May 12, 2025 14:45

sumitarora added 2 commits May 13, 2025 11:27

refactor(devtools): fixing PR comments and adding unit tests

ea2938a

- Update code to fix PR comments and cleanup code - Add unit tests for the new code

sumitarora force-pushed the feat-route-details branch from 12373fb to ea2938a Compare May 13, 2025 15:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(devtools): adding route details panel for selected route #59999

refactor(devtools): adding route details panel for selected route #59999

		const router = getInjectorInstance(rootInjector!, routerInstance[0]);
		return router;


		export type RowType = 'text' \| 'chip' \| 'flag' \| 'list';

		@Component({

	<mat-checkbox (change)="togglePathSettings()">Show Full Path </mat-checkbox>
	<mat-checkbox (change)="togglePathSettings()">Show Full Path</mat-checkbox>

	const rootInjector = (forest[0].resolutionPath ?? []).find((i) => i.name === 'Root');
	const rootInjector = forest[0].resolutionPath?.find((i) => i.name === 'Root');

		const serializedProviderRecords = getSerializedProviderRecords(rootInjector) ?? [];
		const routerInstance = serializedProviderRecords.find(

refactor(devtools): adding route details panel for selected route #59999

Are you sure you want to change the base?

refactor(devtools): adding route details panel for selected route #59999

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment