feat: Refactor routing and layout components for improved navigation …

[PhantomDave]

…and structure

[Copilot]

Pull request overview

This PR refactors the application's routing structure to introduce a hierarchical navigation system with grouped routes under Home, Movements, and Analytics sections. It adds breadcrumb navigation and collapsible side navigation panels, moving from a flat route structure to nested routes with layout components.

Key changes:

• Introduced layout components for organizing routes into logical sections (Home, Movements, Analytics)
• Added breadcrumb component for visual navigation hierarchy
• Refactored side navigation to use Material expansion panels with automatic state management based on current route
• Restructured routing from flat paths (/home, /movements, /balance) to nested paths (/home/dashboard, /movements/tracking, /analytics/monthly-comparison)

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
frontend/src/app/layouts/home-layout/home-layout.component.ts	New layout component for Home section routes (contains only router-outlet)
frontend/src/app/layouts/movements-layout/movements-layout.component.ts	New layout component for Movements section routes (contains only router-outlet)
frontend/src/app/layouts/analytics-layout/analytics-layout.component.ts	New layout component for Analytics section routes (contains only router-outlet)
frontend/src/app/components/ui-library/breadcrumb/breadcrumb.component.ts	New breadcrumb component that dynamically builds navigation trail from route data
frontend/src/app/components/ui-library/breadcrumb/breadcrumb.component.html	Template for breadcrumb display using Material chips with navigation links
frontend/src/app/components/side-nav-component/side-nav-component.ts	Refactored to use expansion panels with signals tracking expanded state per section
frontend/src/app/components/side-nav-component/side-nav-component.html	Restructured navigation with Material expansion panels, added breadcrumb integration
frontend/src/app/components/side-nav-component/side-nav-component.css	Updated styles for expansion panels, toolbar positioning, and breadcrumb placement
frontend/src/app/app.routes.ts	Restructured routes into nested configuration with breadcrumb data on all routes

[Copilot]

These three layout components (HomeLayoutComponent, MovementsLayoutComponent, and AnalyticsLayoutComponent) are identical except for their selector names. This is duplicated code that violates the DRY principle.

Consider creating a single generic layout component or removing these wrapper components entirely since they only contain a <router-outlet />. If the layouts are meant to differ in the future, consider adding a comment explaining the intended differentiation.

[Copilot]

These three layout components (HomeLayoutComponent, MovementsLayoutComponent, and AnalyticsLayoutComponent) are identical except for their selector names. This is duplicated code that violates the DRY principle.

Consider creating a single generic layout component or removing these wrapper components entirely since they only contain a <router-outlet />. If the layouts are meant to differ in the future, consider adding a comment explaining the intended differentiation.

Suggested change

	import { Component } from '@angular/core';
	import { RouterOutlet } from '@angular/router';
	@Component({
	selector: 'app-movements-layout',
	imports: [RouterOutlet],
	template: '<router-outlet />',
	})
	export class MovementsLayoutComponent {}

[Copilot]

These three layout components (HomeLayoutComponent, MovementsLayoutComponent, and AnalyticsLayoutComponent) are identical except for their selector names. This is duplicated code that violates the DRY principle.

Consider creating a single generic layout component or removing these wrapper components entirely since they only contain a <router-outlet />. If the layouts are meant to differ in the future, consider adding a comment explaining the intended differentiation.

Suggested change

	})
	})
	// This layout component is intentionally minimal to allow for future differentiation of the analytics section.

[Copilot]

This clickable element uses tabindex="0" role="button" but is missing keyboard event handling. Elements with role="button" should respond to both Enter and Space key presses for accessibility compliance.

Add a (keydown) handler:

<a mat-list-item (click)="onToggleTheme()" (keydown.enter)="onToggleTheme()" (keydown.space)="onToggleTheme(); $event.preventDefault()" tabindex="0" role="button">

The same issue exists on line 132.

[Copilot]

This clickable element uses tabindex="0" role="button" but is missing keyboard event handling. Elements with role="button" should respond to both Enter and Space key presses for accessibility compliance.

Add a (keydown) handler:

<a mat-list-item (click)="onLogout()" (keydown.enter)="onLogout()" (keydown.space)="onLogout(); $event.preventDefault()" tabindex="0" role="button">

Suggested change

	<a mat-list-item (click)="onLogout()" tabindex="0" role="button">
	<a mat-list-item (click)="onLogout()" (keydown.enter)="onLogout()" (keydown.space)="onLogout(); $event.preventDefault()" tabindex="0" role="button">

[Copilot]

This subscription in the constructor is never unsubscribed, which can lead to a memory leak. When the component is destroyed, the subscription will continue to exist.

In Angular with signals and zoneless change detection, you should use takeUntilDestroyed() from @angular/core/rxjs-interop to automatically clean up subscriptions when the component is destroyed. This is the recommended pattern for the modern Angular approach.

Example fix:

import { takeUntilDestroyed } from '@angular/core/rxjs-interop';

constructor() {
  this.router.events
    .pipe(
      filter((event) => event instanceof NavigationEnd),
      takeUntilDestroyed()
    )
    .subscribe(() => {
      this.breadcrumbs.set(this.createBreadcrumbs(this.activatedRoute.root));
    });
  
  // Initial load
  this.breadcrumbs.set(this.createBreadcrumbs(this.activatedRoute.root));
}

[Copilot]

This subscription is never unsubscribed, which can lead to a memory leak. When the component is destroyed, the subscription will continue to exist.

Since this is in ngOnInit rather than the constructor, you should use takeUntilDestroyed() from @angular/core/rxjs-interop by injecting a DestroyRef and passing it to the function. Alternatively, move this subscription to the constructor where takeUntilDestroyed() works without parameters.

Example fix (moving to constructor):

import { takeUntilDestroyed } from '@angular/core/rxjs-interop';

constructor() {
  // Set initial state based on current route
  this.updateExpandedPanels(this.router.url);

  // Listen to route changes and update expanded panels
  this.router.events
    .pipe(
      filter((event): event is NavigationEnd => event instanceof NavigationEnd),
      takeUntilDestroyed()
    )
    .subscribe((event: NavigationEnd) => {
      this.updateExpandedPanels(event.urlAfterRedirects);
    });
}

Then remove the ngOnInit method entirely.

[PhantomDave]

@copilot open a new pull request to apply changes based on the comments in this thread

[Copilot]

@PhantomDave I've opened a new pull request, #95, to work on those changes. Once the pull request is ready, I'll request review from you.