Skip to content

New starter apps#169

Open
danielleroux wants to merge 41 commits into
mainfrom
feat/new-starter-apps
Open

New starter apps#169
danielleroux wants to merge 41 commits into
mainfrom
feat/new-starter-apps

Conversation

@danielleroux

Copy link
Copy Markdown
Collaborator

Integrate new ix starter apps for angular, react and vue.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors and updates the Angular, React, and Vue starter templates to use the latest versions of Siemens iX (v5) and other core dependencies, while streamlining the project structures and migrating tests to Vitest. The code review feedback focuses on improving the Angular starter by leveraging the built-in routerLinkActive directive instead of a custom route-matching helper, which simplifies the main application component. Additionally, the feedback recommends removing unused imports in the configuration files and implementing safer null/undefined checks when handling form input events in both the Angular and Vue starters.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +11 to +30
<a routerLink="" tabindex="-1">
<ix-menu-item [icon]="icons.iconHome" [attr.active]="isActiveRoute('/') ? '' : null">
Get Started
</ix-menu-item>
</a>
<a routerLink="forms" tabindex="-1">
<ix-menu-item [icon]="icons.iconTextDocument" [attr.active]="isActiveRoute('/forms') ? '' : null">
Forms
</ix-menu-item>
</a>
<a routerLink="charts" tabindex="-1">
<ix-menu-item [icon]="icons.iconPiechart" [attr.active]="isActiveRoute('/charts') ? '' : null">
Charts
</ix-menu-item>
</a>
<a routerLink="grids" tabindex="-1">
<ix-menu-item [icon]="icons.iconTable" [attr.active]="isActiveRoute('/grids') ? '' : null">
Grids
</ix-menu-item>
</a>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Instead of using a custom isActiveRoute helper method with manual URL string matching, we can leverage Angular's built-in routerLinkActive directive. This is more robust, handles query parameters/hash locations automatically, and simplifies the component code.

    <a routerLink="" routerLinkActive #rlaHome="routerLinkActive" [routerLinkActiveOptions]="{exact: true}" tabindex="-1">
      <ix-menu-item [icon]="icons.iconHome" [active]="rlaHome.isActive">
        Get Started
      </ix-menu-item>
    </a>
    <a routerLink="forms" routerLinkActive #rlaForms="routerLinkActive" tabindex="-1">
      <ix-menu-item [icon]="icons.iconTextDocument" [active]="rlaForms.isActive">
        Forms
      </ix-menu-item>
    </a>
    <a routerLink="charts" routerLinkActive #rlaCharts="routerLinkActive" tabindex="-1">
      <ix-menu-item [icon]="icons.iconPiechart" [active]="rlaCharts.isActive">
        Charts
      </ix-menu-item>
    </a>
    <a routerLink="grids" routerLinkActive #rlaGrids="routerLinkActive" tabindex="-1">
      <ix-menu-item [icon]="icons.iconTable" [active]="rlaGrids.isActive">
        Grids
      </ix-menu-item>
    </a>

Comment on lines +1 to +50
import { CommonModule } from '@angular/common';
import { Component } from '@angular/core';
import { Router, RouterLink, RouterOutlet } from '@angular/router';
import {
IxApplication,
IxApplicationHeader,
IxAvatar,
IxContent,
IxMenu,
IxMenuItem,
} from '@siemens/ix-angular/standalone';
import { CompanyLogoComponent } from './../components/company-logo/company-logo.component';
import { iconHome, iconPiechart, iconTable, iconTextDocument } from '@siemens/ix-icons/icons';

@Component({
selector: 'app-root',
imports: [
CommonModule,
RouterOutlet,
RouterLink,
IxApplication,
IxApplicationHeader,
IxAvatar,
IxMenu,
IxMenuItem,
IxContent,
],
templateUrl: './app.html',
styleUrl: './app.css'
})
export class App {

readonly icons = {
iconHome,
iconTextDocument,
iconPiechart,
iconTable
} as const;

protected readonly companyLogoComponent = CompanyLogoComponent;

constructor(private router: Router) {}

isActiveRoute(path: string): boolean {
if (path === '/') {
return this.router.url === '/' || this.router.url === '';
}
return this.router.url === path || this.router.url.startsWith(path + '/');
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

With the adoption of routerLinkActive in the template, we can clean up the component by removing the Router dependency, the constructor, and the custom isActiveRoute method. We also need to import and add RouterLinkActive to the component's imports array.

import { CommonModule } from '@angular/common';
import { Component } from '@angular/core';
import { RouterLink, RouterLinkActive, RouterOutlet } from '@angular/router';
import {
  IxApplication,
  IxApplicationHeader,
  IxAvatar,
  IxContent,
  IxMenu,
  IxMenuItem,
} from '@siemens/ix-angular/standalone';
import { CompanyLogoComponent } from './../components/company-logo/company-logo.component';
import { iconHome, iconPiechart, iconTable, iconTextDocument } from '@siemens/ix-icons/icons';

@Component({
  selector: 'app-root',
  imports: [
    CommonModule,
    RouterOutlet,
    RouterLink,
    RouterLinkActive,
    IxApplication,
    IxApplicationHeader,
    IxAvatar,
    IxMenu,
    IxMenuItem,
    IxContent,
  ],
  templateUrl: './app.html',
  styleUrl: './app.css'
})
export class App {

  readonly icons = {
    iconHome,
    iconTextDocument,
    iconPiechart,
    iconTable
  } as const;

  protected readonly companyLogoComponent = CompanyLogoComponent;
}

import { provideRouter } from '@angular/router';
import { IxModule } from '@siemens/ix-angular';

import { ApplicationConfig, provideZoneChangeDetection, ErrorHandler } from '@angular/core';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The ErrorHandler import is unused in this configuration file and can be safely removed.

Suggested change
import { ApplicationConfig, provideZoneChangeDetection, ErrorHandler } from '@angular/core';
import { ApplicationConfig, provideZoneChangeDetection } from '@angular/core';

Comment thread apps/angular-starter/src/main.ts Outdated
Comment on lines +7 to +9
import { ModuleRegistry, AllCommunityModule } from 'ag-grid-community';
import { addIcons } from '@siemens/ix-icons';
import { iconHome, iconPiechart, iconTextDocument, iconTable, iconQuestion } from '@siemens/ix-icons/icons';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The imports for addIcons and the individual icons are unused in main.ts and can be safely removed.

Suggested change
import { ModuleRegistry, AllCommunityModule } from 'ag-grid-community';
import { addIcons } from '@siemens/ix-icons';
import { iconHome, iconPiechart, iconTextDocument, iconTable, iconQuestion } from '@siemens/ix-icons/icons';
import { ModuleRegistry, AllCommunityModule } from 'ag-grid-community';

Comment on lines +51 to +54
onInspectionTypeChange(event: CustomEvent<string | string[]>) {
const value = event.detail;
this.inspectionType = Array.isArray(value) ? value[0] || '' : value;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To prevent potential runtime type issues or strict null check errors if event.detail is null or undefined, use a nullish coalescing operator to fall back to an empty string.

Suggested change
onInspectionTypeChange(event: CustomEvent<string | string[]>) {
const value = event.detail;
this.inspectionType = Array.isArray(value) ? value[0] || '' : value;
}
onInspectionTypeChange(event: CustomEvent<string | string[]>) {
const value = event.detail;
this.inspectionType = Array.isArray(value) ? value[0] || '' : (value ?? '');
}

label="Inspection Type"
helper-text="Choose the inspection to perform"
:value="inspectionType"
@value-change="inspectionType = String($event.detail)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using String($event.detail) directly will convert null or undefined values to the literal strings "null" or "undefined". It is safer to check if the detail exists before converting it to a string.

      @value-change="inspectionType = $event.detail ? String($event.detail) : ''"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants