-
Notifications
You must be signed in to change notification settings - Fork 13
feat(launchpad): align to new style and responsive design #1236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThe PR adds keyboard navigation to the launchpad (handleAppsNavigation and getVerticalTargetIndex) with layout-aware vertical targeting and RTL support, restructures the launchpad UI (new apps header with close button, title changed to "Switch to application", "Favorites" label), updates markup and styles (app-text layout, SCSS variable imports switched to all-variables, z-index change), introduces Angular animations (expand/backdrop) and no-op animation providers in tests, migrates example code to Signals with categorization and toggles, updates translations/API (removes SI_LAUNCHPAD.SUBTITLE and changes subtitleText input type), and expands unit tests and snapshots. Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant LP as SiLaunchpadFactoryComponent
participant Apps as AppElementsGrid
participant DOM as Browser/Focus
rect rgb(240,248,255)
User->>LP: Press Arrow key / Enter / Esc
end
activate LP
LP->>LP: handleAppsNavigation(event, currentTarget)
LP->>LP: determine key via correctKeyRTL
alt horizontal navigation
LP->>Apps: compute next index (left/right)
else vertical navigation
LP->>Apps: call getVerticalTargetIndex(apps, currentIndex, isUp)
Apps-->>LP: bounding rects (getBoundingClientRect)
end
LP->>DOM: move focus to resolved target element
alt action key (Enter)
DOM->>Apps: activate selected app (click/route)
end
deactivate LP
Note over LP,DOM: Navigation accounts for RTL, row/left tolerances and wraps when needed
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (12)
playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (20)
playwright/e2e/element-examples/si-launchpad.spec.tsplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tsprojects/element-ng/application-header/launchpad/si-launchpad.model.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tssrc/app/examples/si-application-header/si-launchpad.htmlsrc/app/examples/si-application-header/si-launchpad.ts
💤 Files with no reviewable changes (1)
- projects/element-ng/application-header/si-application-header.component.ts
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: spike-rabbit
Repo: siemens/element PR: 1220
File: projects/element-ng/filtered-search/si-filtered-search.component.spec.ts:2602-2733
Timestamp: 2025-12-19T10:37:57.083Z
Learning: For the siemens/element repository, when reviewing PRs, do not duplicate or re-report issues that are already mentioned in the PR comments or objectives. Focus on new findings from the code changes only.
📚 Learning: 2025-12-15T10:05:59.100Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1193
File: projects/element-ng/search-bar/si-search-bar.component.spec.ts:21-28
Timestamp: 2025-12-15T10:05:59.100Z
Learning: In tests that rely on Jasmine's fake clock to control time-based RxJS operators (e.g., debounceTime), use jasmine.clock().install() together with jasmine.clock().mockDate() to enable precise timing control. Without mockDate, scheduler-based operators may not respond to the fake clock. Apply this guidance to spec files (e.g., any file named *.spec.ts or *.spec.js) that test time-sensitive behavior, ensuring you install and mockDate before advancing time in tests.
Applied to files:
playwright/e2e/element-examples/si-launchpad.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.ts
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.
Applied to files:
playwright/e2e/element-examples/si-launchpad.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad.model.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tssrc/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2025-12-16T16:19:17.950Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1207
File: playwright/snapshots/si-filtered-search.spec.ts-snapshots/si-filtered-search--si-filtered-search-playground--data-entered.yaml:38-38
Timestamp: 2025-12-16T16:19:17.950Z
Learning: Do not review files in the playwright/snapshots directory, as they contain auto-generated test artifacts. Exclude any YAML snapshot files under playwright/snapshots from code reviews in the siemens/element repository. If a YAML file is not an auto-generated snapshot, verify its origin before skipping.
Applied to files:
playwright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yaml
📚 Learning: 2025-12-17T14:01:05.705Z
Learnt from: dauriamarco
Repo: siemens/element PR: 908
File: projects/element-ng/side-panel/si-side-panel.component.ts:70-74
Timestamp: 2025-12-17T14:01:05.705Z
Learning: In the Element side-panel component (projects/element-ng/side-panel/si-side-panel.component.ts), the `xxxlMinimum` breakpoint (1920px) is intentionally hardcoded as a private static constant rather than being part of the configurable `Breakpoints` interface. This design preserves full Bootstrap compatibility (which only defines breakpoints up to xxl/1400px) while allowing Element to handle ultra-wide screens explicitly without affecting the standard breakpoint configuration.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-03T13:02:34.875Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1058
File: docs/fundamentals/layouts/breakpoints.md:25-31
Timestamp: 2025-12-03T13:02:34.875Z
Learning: In Element documentation for CSS breakpoints and class infixes (e.g., in docs/fundamentals/layouts/breakpoints.md), _None_ is formatted with italics to indicate the absence of a class infix (representing base mobile-first styles), while actual CSS class infixes like `sm`, `md`, `lg`, `xl`, `xxl` use inline code formatting with backticks. This semantic formatting distinction is intentional.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.html
📚 Learning: 2025-12-04T11:54:31.132Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1133
File: projects/element-ng/tour/si-tour.service.spec.ts:21-26
Timestamp: 2025-12-04T11:54:31.132Z
Learning: In the siemens/element repository, all components are standalone by default and do not require the explicit `standalone: true` flag. Components should be added to the `imports` array in TestBed configuration, not the `declarations` array.
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2025-12-09T14:33:54.441Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: src/app/examples/si-chat-messages/si-ai-message.ts:24-43
Timestamp: 2025-12-09T14:33:54.441Z
Learning: In src/app/examples/ (example/demo code) directories, allow and expect code duplication. Treat duplication as acceptable for demonstration code that aims to illustrate usage rather than enforce DRY. Do not enforce refactoring to remove duplication in example files; focus on readability and clarity of examples. This applies to all TypeScript files under src/app/examples (including nested subdirectories like si-chat-messages).
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (28)
projects/element-ng/application-header/testing/si-launchpad.harness.ts (1)
19-21: LGTM! Text label update aligns with the PR's goal.The category name change from 'Favorite apps' to 'Favorites' is consistent with the broader UI text updates in this refactoring. The test expectations have been updated accordingly.
projects/element-ng/application-header/launchpad/si-launchpad.spec.ts (1)
115-115: LGTM! Test expectation correctly updated.The test assertion now expects 'Favorites' instead of 'Favorite apps', which aligns with the harness update and the PR's UI text changes.
projects/element-ng/application-header/launchpad/si-launchpad.model.ts (1)
51-52: LGTM! New disabled property is well-defined.The addition of the optional
disabledproperty toAppBaseenables apps to be marked as non-interactable. The property is:
- Properly typed as optional boolean
- Documented with clear JSDoc
- Backward compatible (optional property)
This aligns with the component implementation that handles disabled state.
playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yaml (1)
1-31: Skipping review of auto-generated snapshot file.This file contains auto-generated Playwright test snapshots and should not be manually reviewed. Based on learnings, snapshot files under the
playwright/snapshotsdirectory are test artifacts that are automatically updated when tests run.playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yaml (1)
1-23: Skipping review of auto-generated snapshot file.This file contains auto-generated Playwright test snapshots and should not be manually reviewed. Based on learnings, snapshot files under the
playwright/snapshotsdirectory are test artifacts.playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yaml (1)
1-25: Skipping review of auto-generated snapshot file.This file contains auto-generated Playwright test snapshots and should not be manually reviewed. Based on learnings, snapshot files under the
playwright/snapshotsdirectory are test artifacts.projects/element-ng/application-header/testing/si-launchpad-category.harness.ts (1)
18-18: No action needed. The selector.si-h4.mb-6accurately reflects the template markup where category name elements consistently have both classes applied together.playwright/e2e/element-examples/si-launchpad.spec.ts (1)
16-16: Verify the test targets the active app in the launchpad.The favorite icon click now targets
Fischbachinstead ofRocket. SinceFischbachis marked as the active application in the test data (active: true), this change appears intentional—testing the favorite toggle on the active app is more meaningful. The.first()selector is appropriate if multipleFischbachelements appear in the DOM. Confirm the test validates the expected favorite state change captured in the'new favorite'snapshot.projects/element-ng/application-header/launchpad/si-launchpad-app.component.ts (1)
34-35: LGTM! Clean disabled state implementation.The disabled input and host binding follow established patterns in the component, and the runtime guards correctly prevent user interactions when the app is disabled. The implementation is consistent with Angular best practices for OnPush change detection.
Also applies to: 49-50, 60-61, 68-69
projects/element-ng/application-header/launchpad/si-launchpad-app.component.html (1)
6-18: LGTM! Well-structured responsive layout.The template refactoring introduces a cleaner layout structure with proper overflow handling and responsive design support. The use of
text-truncatewithmw-100prevents text overflow issues, and the flex-based layout aligns with the responsive design objectives of this PR.src/app/examples/si-application-header/si-launchpad.html (2)
1-79: Well-structured configuration UI.The new Launchpad Configuration panel provides a clear and user-friendly interface for testing different display modes. The layout uses semantic HTML with proper Bootstrap classes, accessible form controls, and helpful descriptive text.
82-86: Signal-based binding migration is correct.The template properly uses signal function calls with
[apps]="apps()"and[enableFavorites]="enableFavorites()". The TypeScript component correctly defines these as signals:enableFavorites = signal(true)andbaseApps = signal<App[]>([...]), withappsderived as a computed signal. The component importssignalandcomputedfrom@angular/coreand usesChangeDetectionStrategy.OnPush, which aligns with Angular's signal-based reactive state management.projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss (3)
2-2: Theme variable migration is consistent.The import change from
variablestoall-variablesaligns with the systematic theme token updates throughout the file. This ensures access to the complete set of theme variables.
27-31: Proper disabled state styling.The disabled state implementation using
pointer-events: noneandopacity: 0.5follows standard accessibility patterns and correctly prevents interaction while providing visual feedback.
75-100: Well-implemented responsive design.The responsive breakpoint for medium and smaller screens effectively transforms the layout from vertical to horizontal, with appropriate spacing and positioning adjustments. The use of
:has(.favorite-icon)for conditional padding is a modern and clean approach.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html (3)
10-26: Clean header restructuring with accessibility.The new
apps-headerstructure logically groups the title, subtitle, and close button. The close button includes a properaria-labelfor accessibility, and per the project's learning, thesi-iconcomponent automatically handlesaria-hiddeninternally.
49-49: Consistent disabled binding implementation.The
[disabled]="!!app.disabled"binding is correctly applied to both router-link and default anchor cases, ensuring consistent disabled state handling across different app types. The double negation safely coerces the optional boolean to a definite boolean value.Also applies to: 75-75
30-35: Improved semantic structure for categories.Changing the category wrapper from a
spanto adivwith proper block-level styling (mb-6, conditionalmt-*) improves the semantic structure and provides better control over spacing between categories.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (3)
60-63: LGTM! Title text updated consistently.The title text change from "Launchpad" to "Switch to application" is properly reflected in both the JSDoc and implementation. The localization key is preserved for backward compatibility.
86-89: LGTM! Favorite section label simplified.The text change from "Favorite apps" to "Favorites" is concise and properly reflected in both the JSDoc and implementation. The localization key is preserved for backward compatibility.
67-69: No action needed. The subtitle input change is implemented correctly with proper template guards.The optional
subtitleTextinput is appropriately handled in the template using the@if (subtitleText())guard at line 13, which prevents rendering when the value is undefined. This follows Angular best practices for optional signal inputs and ensures clean behavior when no subtitle is provided.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss (4)
8-23: LGTM! Variable references updated consistently.All variable references have been properly updated to use the
all-variablesprefix throughout the file, including spacing, z-index, colors, and elevation variables.
17-27: LGTM! Responsive padding properly implemented.The padding declarations use CSS logical properties and implement responsive behavior:
- Desktop: Different block (6) and inline (8) padding for optimal spacing
- Mobile/tablet (md-down): Uniform padding (6) for simplified layout
This aligns with the PR's objective of introducing responsive design.
30-34: LGTM! Header layout properly defined.The new
.apps-headerclass provides a flexbox layout with space-between justification, appropriate for a header with title and action button (e.g., close button) on opposite ends.
3-3: All-variables module is correctly configured and exports all required symbols.The import has been changed from
variablestoall-variables, which is the comprehensive module that forwards all symbol definitions including SI variables (viavariables), bootstrap functions, variables, mixins, and utilities. All symbols used in this file—such as$si-application-header-height,$spacers,$zindex-launchpad,$element-base-1,$element-elevation-inset-1, and themedia-breakpoint-downmixin—are properly exported and available through this module.src/app/examples/si-application-header/si-launchpad.ts (3)
5-5: LGTM! Signal-based reactive state properly implemented.The component now uses Angular signals for reactive state management:
- Imports
signalandcomputedfrom @angular/core- Imports
AppCategoryfor typed category support- Declares
enableFavoritesandenableCategoriessignals with appropriate defaultsThis aligns with modern Angular reactive patterns.
Also applies to: 9-9, 29-30
32-120: LGTM! Comprehensive example data with signal-based state.The app list has been converted to a
baseAppssignal with extended example data demonstrating various app states (favorite, active, external, disabled). The duplicate "App name" and "System name" labels are acceptable for demonstration purposes.Based on learnings, code duplication in example files is expected to illustrate usage clearly.
122-162: LGTM! Categorization logic correctly implemented.The
appscomputed signal properly implements conditional categorization:
- Returns flat list when categories are disabled
- Groups apps into Business Applications, System Tools, and Other Applications when enabled
- Empty categories are excluded from the result
- Return type correctly typed as
App[] | AppCategory[]The hardcoded category filters are appropriate for demonstration code.
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss
Outdated
Show resolved
Hide resolved
5eddf56 to
e5fa7a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (1)
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss (1)
40-45: Past review comment still applies.This code segment was previously flagged for using
!important. The maintainability concern about!importantremains valid.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (12)
playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (21)
playwright/e2e/element-examples/si-launchpad.spec.tsplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tsprojects/element-ng/application-header/launchpad/si-launchpad.model.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/translate/si-translatable-keys.interface.tssrc/app/examples/si-application-header/si-launchpad.htmlsrc/app/examples/si-application-header/si-launchpad.ts
💤 Files with no reviewable changes (2)
- projects/element-ng/translate/si-translatable-keys.interface.ts
- projects/element-ng/application-header/si-application-header.component.ts
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: spike-rabbit
Repo: siemens/element PR: 1220
File: projects/element-ng/filtered-search/si-filtered-search.component.spec.ts:2602-2733
Timestamp: 2025-12-19T10:37:57.083Z
Learning: For the siemens/element repository, when reviewing PRs, do not duplicate or re-report issues that are already mentioned in the PR comments or objectives. Focus on new findings from the code changes only.
📚 Learning: 2025-12-16T16:19:17.950Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1207
File: playwright/snapshots/si-filtered-search.spec.ts-snapshots/si-filtered-search--si-filtered-search-playground--data-entered.yaml:38-38
Timestamp: 2025-12-16T16:19:17.950Z
Learning: Do not review files in the playwright/snapshots directory, as they contain auto-generated test artifacts. Exclude any YAML snapshot files under playwright/snapshots from code reviews in the siemens/element repository. If a YAML file is not an auto-generated snapshot, verify its origin before skipping.
Applied to files:
playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yaml
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad.model.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsplaywright/e2e/element-examples/si-launchpad.spec.tssrc/app/examples/si-application-header/si-launchpad.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-22T13:04:20.883Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: src/app/examples/ag-grid/ag-grid-empty-state.ts:20-24
Timestamp: 2025-12-22T13:04:20.883Z
Learning: The SiEmptyStateComponent in siemens/element-ng accepts icon names in kebab-case format (e.g., 'element-technical-operator') that may not be directly exported as constants in the element-icons.ts file. Do not flag these as errors if they render correctly at runtime.
Applied to files:
projects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlsrc/app/examples/si-application-header/si-launchpad.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-17T14:01:05.705Z
Learnt from: dauriamarco
Repo: siemens/element PR: 908
File: projects/element-ng/side-panel/si-side-panel.component.ts:70-74
Timestamp: 2025-12-17T14:01:05.705Z
Learning: In the Element side-panel component (projects/element-ng/side-panel/si-side-panel.component.ts), the `xxxlMinimum` breakpoint (1920px) is intentionally hardcoded as a private static constant rather than being part of the configurable `Breakpoints` interface. This design preserves full Bootstrap compatibility (which only defines breakpoints up to xxl/1400px) while allowing Element to handle ultra-wide screens explicitly without affecting the standard breakpoint configuration.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-03T13:02:34.875Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1058
File: docs/fundamentals/layouts/breakpoints.md:25-31
Timestamp: 2025-12-03T13:02:34.875Z
Learning: In Element documentation for CSS breakpoints and class infixes (e.g., in docs/fundamentals/layouts/breakpoints.md), _None_ is formatted with italics to indicate the absence of a class infix (representing base mobile-first styles), while actual CSS class infixes like `sm`, `md`, `lg`, `xl`, `xxl` use inline code formatting with backticks. This semantic formatting distinction is intentional.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-15T10:05:59.100Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1193
File: projects/element-ng/search-bar/si-search-bar.component.spec.ts:21-28
Timestamp: 2025-12-15T10:05:59.100Z
Learning: In tests that rely on Jasmine's fake clock to control time-based RxJS operators (e.g., debounceTime), use jasmine.clock().install() together with jasmine.clock().mockDate() to enable precise timing control. Without mockDate, scheduler-based operators may not respond to the fake clock. Apply this guidance to spec files (e.g., any file named *.spec.ts or *.spec.js) that test time-sensitive behavior, ensuring you install and mockDate before advancing time in tests.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad.spec.tsplaywright/e2e/element-examples/si-launchpad.spec.ts
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.ts
📚 Learning: 2025-12-04T11:54:31.132Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1133
File: projects/element-ng/tour/si-tour.service.spec.ts:21-26
Timestamp: 2025-12-04T11:54:31.132Z
Learning: In the siemens/element repository, all components are standalone by default and do not require the explicit `standalone: true` flag. Components should be added to the `imports` array in TestBed configuration, not the `declarations` array.
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2025-12-09T14:33:54.441Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: src/app/examples/si-chat-messages/si-ai-message.ts:24-43
Timestamp: 2025-12-09T14:33:54.441Z
Learning: In src/app/examples/ (example/demo code) directories, allow and expect code duplication. Treat duplication as acceptable for demonstration code that aims to illustrate usage rather than enforce DRY. Do not enforce refactoring to remove duplication in example files; focus on readability and clarity of examples. This applies to all TypeScript files under src/app/examples (including nested subdirectories like si-chat-messages).
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (29)
projects/element-ng/application-header/testing/si-launchpad.harness.ts (1)
19-21: LGTM! Text label updated consistently.The category name change from 'Favorite apps' to 'Favorites' correctly aligns with the UI text updates across the PR.
projects/element-ng/application-header/launchpad/si-launchpad.spec.ts (1)
115-115: LGTM! Test expectation updated correctly.The test now validates the updated category name 'Favorites', maintaining consistency with the UI label changes.
projects/element-ng/application-header/launchpad/si-launchpad.model.ts (1)
51-52: LGTM! Clean interface extension.The optional
disabledproperty is well-documented and extends the app model without breaking changes. The implementation correctly propagates to all app types (AppLink and AppRouterLink).projects/element-ng/application-header/launchpad/si-launchpad-app.component.ts (3)
49-50: LGTM! Input property correctly configured.The
disabledinput is properly implemented withbooleanAttributetransform, consistent with other boolean inputs in the component.
34-35: LGTM! Host bindings applied correctly.Both
[class.action]and[class.disabled]host bindings are properly configured to reflect component state.
58-65: LGTM! Disabled state guard prevents favorite toggle interaction.The early-exit guard correctly prevents favorite toggling when the app is disabled.
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss (6)
1-2: LGTM! Theme import updated to all-variables.The migration from
variablestoall-variablesaligns with the broader theme token modernization across the PR.
4-25: LGTM! Token references migrated and selectors optimized.The token references are correctly updated to use
all-variablesequivalents, and the combined&.active, &:hoverselector is cleaner than separate blocks.
33-39: LGTM! New container supports improved layout structure.The
.app-textcontainer with flex column layout and overflow handling provides proper structure for the text content and integrates well with the responsive design.
41-56: LGTM! Icon tokens migrated consistently.All
.app-icontoken references are correctly updated to useall-variablesequivalents, maintaining the existing layout and styling.
58-73: LGTM! Favorite and external icon styling updated correctly.Token migrations for
.favorite-icon,.external-icon, and.is-favoriteare consistent with the all-variables theme system.
75-100: LGTM! Responsive design implementation is well-structured.The responsive layout for md and down breakpoints correctly switches from vertical to horizontal layout, with appropriate spacing and positioning adjustments. The use of
:has(.favorite-icon)for conditional padding is elegant.projects/element-ng/application-header/launchpad/si-launchpad-app.component.html (1)
6-18: LGTM! Clean template restructuring.The new layout groups the app name and external icon in a flex row, with the system name below—a clearer visual hierarchy. Typography classes (si-h4, si-body) and utility classes are appropriate.
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss (3)
3-3: LGTM! Import aligned with all-variables.Switching to
all-variablesprovides access to the complete variable set and is the recommended approach for component styling.
30-34: LGTM! Header flex layout is appropriate.The new
.apps-headerblock provides a clean flex container for the header content with proper alignment and spacing.
25-27: LGTM! Responsive padding improves mobile UX.Reducing padding on smaller screens (medium breakpoint and below) is a good responsive design practice.
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html (3)
10-25: LGTM! Well-structured header with proper accessibility.The new
apps-headerblock groups the title, optional subtitle, and close button together. The close button includes a properaria-labelfor accessibility, and the conditional rendering ensures clean display when subtitle is not provided.
49-49: LGTM! Disabled state properly bound.The
[disabled]="!!app.disabled"binding correctly converts the optional disabled property to a boolean for both router-link and default anchor cases.Also applies to: 75-75
30-34: LGTM! Improved semantic structure for category headers.Changing the category name wrapper from
spantodivis more semantic for block-level content. The conditional margin classes (mt-4for first,mt-8for others) provide appropriate spacing.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (2)
60-69: LGTM! Text updates align with design refresh.The updated localization keys reflect the new UI text: "Switch to application" for the title and making the subtitle optional. The use of
t(() => $localize(...))ensures proper i18n support.
86-89: LGTM! Favorites text updated consistently.The shortened "Favorites" text aligns with the design refresh and maintains proper localization support.
playwright/e2e/element-examples/si-launchpad.spec.ts (1)
16-16: LGTM! Test updated to match new example data.The test now targets the Fischbach item for favorite toggling, aligning with the updated example data. The use of semantic role selectors supports accessibility validation.
src/app/examples/si-application-header/si-launchpad.html (3)
1-7: LGTM!The wrapper structure with
has-navbar-fixed-topis properly implemented for the fixed header layout.
8-78: LGTM!The Launchpad Configuration panel is well-structured with:
- Proper signal binding syntax using parentheses for signal accessors
- Accessible form switches with correct labels and
role="switch"- Responsive layout with appropriate Bootstrap grid classes
81-87: LGTM!The template correctly binds to the computed signals using parentheses to invoke the signal accessors.
src/app/examples/si-application-header/si-launchpad.ts (4)
5-5: LGTM!The reactive primitives (
computed,signal) andAppCategorytype are correctly imported for the new signal-based architecture.Also applies to: 9-9
29-30: LGTM!The public signals for
enableFavoritesandenableCategoriesare correctly declared with sensible defaults.
32-120: LGTM!The
baseAppssignal is properly initialized with demonstration data showcasing various app states (favorite, active, external, disabled). The repetitive naming is appropriate for demo/example code.
169-177: LGTM!The toggle methods correctly cast the event target to
HTMLInputElementand update the corresponding signals with the checked state.
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
Outdated
Show resolved
Hide resolved
projects/element-ng/application-header/launchpad/si-launchpad-app.component.ts
Show resolved
Hide resolved
dc02ce4 to
9b05161
Compare
|
Documentation. Coverage Reports: |
8a98926 to
43d071b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (2)
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss (1)
39-44: Previously flagged: Consider alternatives to!important.This has been noted in a prior review. The
!importantusage here may be necessary due to specificity conflicts with sibling selector timing, but alternatives like increased specificity or restructured selectors could be explored if this becomes a maintenance concern.src/app/examples/si-application-header/si-launchpad.ts (1)
164-167: Direct mutation triggers reactivity via spread.This pattern (mutate then
set([...array])) works because the spread creates a new array reference, triggering signal subscribers. A previous review suggested an immutable approach, which remains a valid alternative for stricter functional patterns.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (12)
playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (29)
api-goldens/element-ng/application-header/index.api.mdapi-goldens/element-ng/translate/index.api.mdplaywright/e2e/element-examples/si-launchpad.spec.tsplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tsprojects/element-ng/application-header/launchpad/si-launchpad.model.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.htmlprojects/element-ng/application-header/si-application-header.component.scssprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/translate/si-translatable-keys.interface.tsprojects/element-theme/src/styles/components/_application-header.scssprojects/element-theme/src/styles/variables/_zindex.scsssrc/app/examples/si-application-header/si-launchpad.htmlsrc/app/examples/si-application-header/si-launchpad.ts
💤 Files with no reviewable changes (2)
- api-goldens/element-ng/translate/index.api.md
- projects/element-ng/translate/si-translatable-keys.interface.ts
🧰 Additional context used
🧠 Learnings (28)
📓 Common learnings
Learnt from: spike-rabbit
Repo: siemens/element PR: 1220
File: projects/element-ng/filtered-search/si-filtered-search.component.spec.ts:2602-2733
Timestamp: 2025-12-19T10:37:57.083Z
Learning: For the siemens/element repository, when reviewing PRs, do not duplicate or re-report issues that are already mentioned in the PR comments or objectives. Focus on new findings from the code changes only.
📚 Learning: 2025-12-16T16:19:17.950Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1207
File: playwright/snapshots/si-filtered-search.spec.ts-snapshots/si-filtered-search--si-filtered-search-playground--data-entered.yaml:38-38
Timestamp: 2025-12-16T16:19:17.950Z
Learning: Do not review files in the playwright/snapshots directory, as they contain auto-generated test artifacts. Exclude any YAML snapshot files under playwright/snapshots from code reviews in the siemens/element repository. If a YAML file is not an auto-generated snapshot, verify its origin before skipping.
Applied to files:
playwright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yaml
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-22T13:04:20.883Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: src/app/examples/ag-grid/ag-grid-empty-state.ts:20-24
Timestamp: 2025-12-22T13:04:20.883Z
Learning: The SiEmptyStateComponent in siemens/element-ng accepts icon names in kebab-case format (e.g., 'element-technical-operator') that may not be directly exported as constants in the element-icons.ts file. Do not flag these as errors if they render correctly at runtime.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scsssrc/app/examples/si-application-header/si-launchpad.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsplaywright/e2e/element-examples/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/si-application-header.component.spec.tssrc/app/examples/si-application-header/si-launchpad.tsprojects/element-ng/application-header/launchpad/si-launchpad.model.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-15T10:05:59.100Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1193
File: projects/element-ng/search-bar/si-search-bar.component.spec.ts:21-28
Timestamp: 2025-12-15T10:05:59.100Z
Learning: In tests that rely on Jasmine's fake clock to control time-based RxJS operators (e.g., debounceTime), use jasmine.clock().install() together with jasmine.clock().mockDate() to enable precise timing control. Without mockDate, scheduler-based operators may not respond to the fake clock. Apply this guidance to spec files (e.g., any file named *.spec.ts or *.spec.js) that test time-sensitive behavior, ensuring you install and mockDate before advancing time in tests.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad.spec.tsplaywright/e2e/element-examples/si-launchpad.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-11-26T13:50:02.735Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1090
File: projects/element-ng/typeahead/si-typeahead.directive.ts:511-515
Timestamp: 2025-11-26T13:50:02.735Z
Learning: In the typeahead component (projects/element-ng/typeahead/si-typeahead.directive.ts), the `createOption()` method should always close the dropdown with `this.canBeOpen.set(false)` regardless of whether `typeaheadMultiSelect` is enabled. This is different from `selectMatch()` which only closes the dropdown when NOT in multi-select mode.
Applied to files:
projects/element-ng/application-header/si-application-header.component.ts
📚 Learning: 2025-12-11T10:09:01.564Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1151
File: projects/element-ng/auto-collapsable-list/si-auto-collapsable-list.directive.spec.ts:83-104
Timestamp: 2025-12-11T10:09:01.564Z
Learning: In the auto-collapsable-list directive tests (projects/element-ng/auto-collapsable-list/si-auto-collapsable-list.directive.spec.ts), jasmine.clock() successfully controls the directive's setTimeout-based layout logic in zoneless mode. Mock timers work fine here, unlike in select component overflow tests where real setTimeout waits are required.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-08T11:24:45.272Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/options/si-select-lazy-options.directive.spec.ts:67-85
Timestamp: 2025-12-08T11:24:45.272Z
Learning: In select lazy options tests (projects/element-ng/select/options/si-select-lazy-options.directive.spec.ts), jasmine.clock() cannot control RxJS debounceTime in zoneless mode. Real setTimeout waits must be used instead for tests involving search debouncing.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T07:16:53.762Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts:92-105
Timestamp: 2025-12-15T07:16:53.762Z
Learning: In gridstack-wrapper component tests (projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts), real setTimeout waits must be used instead of fixture.whenStable() to avoid injector destroyed errors during mount/unmount operations in zoneless mode. The GridStack timing-dependent operations require actual async delays.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-08T11:25:51.584Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/si-select.component.spec.ts:511-516
Timestamp: 2025-12-08T11:25:51.584Z
Learning: In the select component overflow tests (projects/element-ng/select/si-select.component.spec.ts), jasmine.clock() cannot be used to control the timing for overflow item detection. Real setTimeout waits must be used instead, likely due to ResizeObserver or debounced layout calculations that jasmine.clock() cannot control in zoneless mode.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T11:54:31.132Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1133
File: projects/element-ng/tour/si-tour.service.spec.ts:21-26
Timestamp: 2025-12-04T11:54:31.132Z
Learning: In the siemens/element repository, all components are standalone by default and do not require the explicit `standalone: true` flag. Components should be added to the `imports` array in TestBed configuration, not the `declarations` array.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tssrc/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2025-12-08T11:25:20.861Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/si-select.component.spec.ts:204-205
Timestamp: 2025-12-08T11:25:20.861Z
Learning: In zoneless Angular tests (using provideZonelessChangeDetection), calling fixture.changeDetectorRef.markForCheck() before fixture.detectChanges() is required even for components using the default change detection strategy (not OnPush). This differs from zone.js-based testing where markForCheck is primarily needed only for OnPush components.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T05:50:17.637Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1135
File: projects/element-ng/tree-view/si-tree-view.component.spec.ts:953-964
Timestamp: 2025-12-04T05:50:17.637Z
Learning: In the siemens/element repository, when making tree-view tests zoneless-ready, jasmine.clock() should be used to mimic setTimeouts even in zoneless mode, as the component implementation relies on setTimeout calls that need to be controlled in tests.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T05:50:38.735Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1135
File: projects/element-ng/tree-view/si-tree-view.component.spec.ts:1244-1245
Timestamp: 2025-12-04T05:50:38.735Z
Learning: In zoneless Angular tests with OnPush components, calling `await runOnPushChangeDetection(fixture)` followed by `await fixture.whenStable()` is NOT redundant. The `runOnPushChangeDetection` helper forces change detection to run for OnPush components, while `whenStable()` waits for all async operations to complete. Both are necessary and serve different purposes.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-02T06:50:25.778Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1104
File: projects/element-ng/datepicker/components/si-day-selection.component.spec.ts:19-19
Timestamp: 2025-12-02T06:50:25.778Z
Learning: The `runOnPushChangeDetection` helper is located at `projects/element-ng/test-helpers/change-detection.helper.ts` and is shared across the element-ng project, not scoped under individual feature modules like datepicker.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.ts
📚 Learning: 2025-12-01T04:01:27.365Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1099
File: projects/element-ng/accordion/si-accordion.component.spec.ts:39-43
Timestamp: 2025-12-01T04:01:27.365Z
Learning: With Angular 20 and esbuild, synchronous `TestBed.configureTestingModule(...).compileComponents()` calls work correctly even when components use external templates (templateUrl), because esbuild pre-compiles templates at build time. The async/await pattern is not required in this scenario.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-01T14:12:11.111Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1104
File: projects/element-ng/datepicker/components/si-day-selection.component.spec.ts:198-202
Timestamp: 2025-12-01T14:12:11.111Z
Learning: In zoneless Angular tests (using provideZonelessChangeDetection), prefer `await fixture.whenStable()` over `fixture.detectChanges()` after dispatching events or triggering async operations, as `whenStable()` waits for all asynchronous operations to complete and better mimics the production environment.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-05T08:00:38.407Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1151
File: projects/element-ng/typeahead/si-typeahead.directive.spec.ts:114-121
Timestamp: 2025-12-05T08:00:38.407Z
Learning: In the typeahead test suite (projects/element-ng/typeahead/si-typeahead.directive.spec.ts), jasmine.clock() must be installed in beforeAll and uninstalled in afterAll (not beforeEach/afterEach) because using per-test clock installation causes schedulerFunction-related issues in these specific tests.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T07:16:32.082Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts:69-80
Timestamp: 2025-12-15T07:16:32.082Z
Learning: In gridstack-wrapper component tests (projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts), real setTimeout waits (e.g., `await new Promise(resolve => setTimeout(resolve, 0))`) must be used instead of `fixture.whenStable()` when testing grid item rendering and initialization. This is likely due to gridstack library's initialization timing or lifecycle behavior that fixture.whenStable() cannot properly wait for in zoneless mode.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-22T13:29:31.048Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/ag-grid/parts/color-scheme.part.ts:35-35
Timestamp: 2025-12-22T13:29:31.048Z
Learning: In the Element design system (projects/element-ng/theme and projects/element-theme), only `--element-body-font-family` is exposed as a CSS custom property for typography. Font sizes are NOT exposed as CSS custom properties; they are defined using internal SCSS variables (e.g., `typography.$si-font-size-body`) that compile to fixed values. Using hardcoded pixel values for fontSize in AG Grid theme configuration (e.g., `fontSize: '14px'`) is the correct approach.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-17T14:01:05.705Z
Learnt from: dauriamarco
Repo: siemens/element PR: 908
File: projects/element-ng/side-panel/si-side-panel.component.ts:70-74
Timestamp: 2025-12-17T14:01:05.705Z
Learning: In the Element side-panel component (projects/element-ng/side-panel/si-side-panel.component.ts), the `xxxlMinimum` breakpoint (1920px) is intentionally hardcoded as a private static constant rather than being part of the configurable `Breakpoints` interface. This design preserves full Bootstrap compatibility (which only defines breakpoints up to xxl/1400px) while allowing Element to handle ultra-wide screens explicitly without affecting the standard breakpoint configuration.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-03T13:02:34.875Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1058
File: docs/fundamentals/layouts/breakpoints.md:25-31
Timestamp: 2025-12-03T13:02:34.875Z
Learning: In Element documentation for CSS breakpoints and class infixes (e.g., in docs/fundamentals/layouts/breakpoints.md), _None_ is formatted with italics to indicate the absence of a class infix (representing base mobile-first styles), while actual CSS class infixes like `sm`, `md`, `lg`, `xl`, `xxl` use inline code formatting with backticks. This semantic formatting distinction is intentional.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-15T07:17:06.981Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/widget-instance-editor-dialog/si-widget-instance-editor-dialog.component.spec.ts:37-49
Timestamp: 2025-12-15T07:17:06.981Z
Learning: In widget-instance-editor-dialog component tests (projects/dashboards-ng/src/components/widget-instance-editor-dialog/si-widget-instance-editor-dialog.component.spec.ts), real setTimeout waits (e.g., `await new Promise(resolve => setTimeout(resolve, 0))`) must be used instead of `fixture.whenStable()` when testing component initialization. This is likely due to the component's lifecycle behavior or modal initialization timing that fixture.whenStable() cannot properly wait for in zoneless mode.
Applied to files:
projects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-09T14:33:54.441Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: src/app/examples/si-chat-messages/si-ai-message.ts:24-43
Timestamp: 2025-12-09T14:33:54.441Z
Learning: In src/app/examples/ (example/demo code) directories, allow and expect code duplication. Treat duplication as acceptable for demonstration code that aims to illustrate usage rather than enforce DRY. Do not enforce refactoring to remove duplication in example files; focus on readability and clarity of examples. This applies to all TypeScript files under src/app/examples (including nested subdirectories like si-chat-messages).
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2025-12-23T09:24:35.163Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/translate/index.api.md:12-84
Timestamp: 2025-12-23T09:24:35.163Z
Learning: In the siemens/element repository, do not review files under the api-goldens/ directory (e.g., api-goldens/**/index.api.md) since they are auto-generated by API Extractor. Exclude these from review checks and focus on source files that are not auto-generated.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-23T09:24:57.843Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/index.api.md:324-325
Timestamp: 2025-12-23T09:24:57.843Z
Learning: Do not review files in the api-goldens directory, as they are auto-generated API reports produced by API Extractor.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-23T09:24:40.395Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/module-federation/index.api.md:7-11
Timestamp: 2025-12-23T09:24:40.395Z
Learning: In the siemens/element repository, do not review any files under the api-goldens directory, as they are auto-generated API reports produced by API Extractor. These MD files should be excluded from review unless explicitly overridden.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
🧬 Code graph analysis (2)
src/app/examples/si-application-header/si-launchpad.ts (1)
projects/element-ng/application-header/launchpad/si-launchpad.model.ts (2)
App(55-55)AppCategory(31-36)
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (1)
projects/element-ng/common/helpers/rtl.ts (1)
correctKeyRTL(9-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (33)
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.ts (1)
15-15: LGTM! Proper test setup for animated components.Adding
provideNoopAnimations()is the correct approach for testing components that use animations. This disables animations during tests, ensuring faster execution and eliminating timing-related flakiness.Also applies to: 51-56
projects/element-theme/src/styles/variables/_zindex.scss (1)
19-19: LGTM! Z-index adjustment correctly positions launchpad below application header.The change ensures the application header (z-index: 1033) appears above the launchpad (z-index: 1032), which aligns with the PR's design where the header now contains launchpad controls like the "Close launchpad" button. The cascading relationship with the backdrop (line 20) is preserved, and the launchpad remains appropriately positioned below modal/offcanvas layers.
projects/element-ng/application-header/testing/si-launchpad-category.harness.ts (1)
18-18: LGTM! Selector correctly updated for style refactoring.The selector change from
.si-h4to.si-h4.mb-6properly aligns with the template updates in this PR's style refactoring work. The harness will correctly target the category name element with the new styling classes.projects/element-ng/application-header/si-application-header.component.spec.ts (1)
15-15: LGTM!Correctly adding
provideNoopAnimations()to disable animations during tests. This ensures deterministic test behavior now that animations are introduced in the component.Also applies to: 106-106
projects/element-ng/application-header/si-application-header.component.ts (2)
5-11: Good practice aliasingtriggerasanimationTrigger.This avoids potential naming conflicts with other identifiers in the codebase.
51-81: Animation definitions look correct.The asymmetric enter/leave timing (0.5s/0.25s) for the expand animation provides a nice UX feel. The backdrop animation properly uses linear easing for opacity transitions.
projects/element-ng/application-header/si-application-header.component.html (1)
55-61: LGTM!The launchpad container with the
[@expand]animation trigger is correctly structured for the enter/leave transitions.projects/element-ng/application-header/si-application-header.component.scss (1)
8-13: Launchpad interactivity is properly ensured.The
.app-switcherinsi-launchpad-factory.component.scssalready haspointer-events: allapplied, which correctly overrides the container'spointer-events: noneand keeps launchpad content interactive.projects/element-ng/application-header/testing/si-launchpad.harness.ts (1)
19-21: LGTM! Text update aligns with UI changes.The update from 'Favorite apps' to 'Favorites' is consistent with the broader launchpad text refactoring across the PR.
projects/element-ng/application-header/launchpad/si-launchpad.model.ts (1)
51-52: LGTM! Clean addition to the public API.The optional
disabledproperty is well-documented and extends the launchpad model to support disabled app states without breaking existing implementations.projects/element-ng/application-header/launchpad/si-launchpad.spec.ts (1)
115-115: LGTM! Test updated to match new category name.The test expectation correctly reflects the category name change from 'Favorite apps' to 'Favorites'.
playwright/e2e/element-examples/si-launchpad.spec.ts (1)
16-16: LGTM! Test updated to match new example data.The test now targets 'Fischbach' instead of 'Rocket' for the favorite toggle interaction, aligning with updated test fixtures or example data.
projects/element-ng/application-header/launchpad/si-launchpad-app.component.ts (1)
34-36: LGTM! Disabled state implementation is complete and correct.The component properly handles the disabled state:
- Host bindings apply appropriate class and remove the element from tab order when disabled
- Early-exit guards in
favoriteClickedandclickprevent interaction when disabled- The
disabledinput follows established patterns withbooleanAttributetransformationThe accessibility concern regarding
tabindexhas been addressed per the previous review comment.Also applies to: 50-51, 61-62, 69-70
projects/element-ng/application-header/launchpad/si-launchpad-app.component.html (1)
6-17: LGTM! Clean restructuring of the app text layout.The new
app-textcontainer with flex column layout properly handles text overflow usingtext-truncateandmw-100on both the app-name and systemName elements. The si-h4 row correctly aligns the name with the optional external icon.playwright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yaml (1)
1-51: Auto-generated snapshot file - no review needed.This file is an auto-generated Playwright test artifact that reflects the updated UI structure. Based on learnings, files in the
playwright/snapshotsdirectory are excluded from code review.api-goldens/element-ng/application-header/index.api.md (1)
30-30: Auto-generated API report - no review needed.This file is auto-generated by API Extractor. The changes correctly reflect the new
disabled?: booleanproperty onAppBaseand the updatedsubtitleTexttype signature. Based on learnings, files in theapi-goldens/directory are excluded from review.playwright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yaml (1)
1-49: Auto-generated snapshot file - no review needed.This Playwright snapshot is auto-generated and reflects the updated launchpad UI with the new favorite state. Based on learnings, this file is excluded from code review.
playwright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yaml (1)
1-51: Auto-generated snapshot file - no review needed.This Playwright snapshot is auto-generated and reflects the mobile responsive layout. Based on learnings, this file is excluded from code review.
src/app/examples/si-application-header/si-launchpad.html (2)
82-86: LGTM! Clean reactive bindings for launchpad factory.The template correctly uses signal-based bindings (
apps(),enableFavorites()) and properly wires thefavoriteChangeoutput. This aligns well with the reactive state management approach introduced in this PR.
31-38: ThetoggleFavoritesmethod correctly extracts the checked state from the DOM event. It properly castsevent.targettoHTMLInputElementand accessestarget.checked, then updates the signal. The implementation is correct and follows proper type safety practices.projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss (2)
2-3: LGTM! Consistent token migration to all-variables.The import change from
variablestoall-variablesaligns with the broader theming refactor across the launchpad components.
75-100: Responsive layout implementation looks solid.The mobile breakpoint correctly:
- Switches to row layout with
flex-flow: row nowrap- Uses
:has(.favorite-icon)for conditional padding adjustment- Realigns
.app-texttoalign-items: start- Repositions the favorite icon appropriately
The
:has()CSS selector has good modern browser support (Chrome 105+, Safari 15.4+, Firefox 121+). Verify this aligns with the project's browser support matrix.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss (2)
29-33: LGTM! Clean header layout.The new
.apps-headerclass provides a straightforward flex layout for the header section with proper spacing between elements.
6-7: The positioning context is properly established via the parent.launchpad-containerwithposition: fixed.The change from
position: fixedtoposition: absoluteworks correctly within this structure. The.app-switcherwill be positioned relative to the.launchpad-containerinstead of the viewport, which is appropriate for this nested layout.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html (3)
9-25: LGTM! Well-structured header with proper accessibility.The new apps-header block cleanly groups the title, optional subtitle, and close button. The close button correctly includes an
aria-labelfor screen readers, and thesi-iconcomponent handlesaria-hiddeninternally (per repository conventions).
49-49: LGTM! Disabled state binding is correct.The
!!app.disabledcoercion ensures a boolean value is always passed to the[disabled]input, handlingundefinedgracefully.Also applies to: 76-76
59-59: LGTM! Keyboard navigation event binding.Using
$event.currentTargetcorrectly references the element that has the event listener attached (the anchor), which is the intended behavior for the navigation logic.Also applies to: 79-79
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (2)
159-199: LGTM! Comprehensive keyboard navigation with RTL support.The navigation logic correctly:
- Uses
correctKeyRTLto handle RTL layouts (swapping left/right arrows)- Guards against non-arrow keys and invalid elements
- Queries only enabled, non-disabled apps via a comprehensive selector
- Wraps horizontally using modulo arithmetic
- Delegates vertical navigation to spatial calculation
- Prevents default to avoid scroll interference
201-276: Well-implemented spatial grid navigation.The vertical navigation logic intelligently handles both single-column (mobile) and multi-column (grid) layouts:
- Caches bounding rects to avoid repeated expensive DOM calls (line 203-206)
- Detects single-column layout by checking if all apps share the same left position
- Uses tolerance-based row grouping for robust comparison
- Finds the horizontally closest element in the target row
One minor observation: the tolerance values (20px for left alignment, 10px for rows) are hardcoded. These work well for typical layouts but could be extracted to constants if they need adjustment later.
src/app/examples/si-application-header/si-launchpad.ts (4)
29-30: LGTM! Clean reactive state management.Using signals for
enableFavoritesandenableCategoriesenables clean two-way binding with the configuration UI and proper reactivity.
32-120: LGTM! Comprehensive sample data for demo purposes.The extended app entries cover various scenarios (favorites, active state, external links, disabled state, long names) which is excellent for demonstrating and testing the launchpad's capabilities.
122-162: Categorization logic works correctly for demo purposes.The hardcoded name matching is appropriate for example code that demonstrates the categorization feature. The implementation correctly builds category arrays conditionally and handles the flat vs. categorized return type.
169-177: LGTM! Toggle handlers are straightforward.The type cast to
HTMLInputElementis appropriate for checkbox change events, and the signal updates will trigger the computedappsto re-evaluate.
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss
Show resolved
Hide resolved
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
Outdated
Show resolved
Hide resolved
projects/element-ng/application-header/si-application-header.component.html
Show resolved
Hide resolved
projects/element-theme/src/styles/components/_application-header.scss
Outdated
Show resolved
Hide resolved
|
@Danisand Please be aware that we'll be switching the title of the launchpad from /cc @michael-smt |
You are right @kfenner we distinguish between
@panch1739 Is there a blocker to continue using |
43d071b to
d1ec321
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
projects/element-ng/application-header/testing/si-launchpad-category.harness.ts (1)
18-18: Selector updated to match template structure.The selector now targets
.si-h4.mb-6to match the updated template structure. While this aligns with the style refactoring, note that a previous review suggested usingdata-test-idattributes for more stable test selectors that are resilient to styling changes.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss (1)
39-44: Note: !important usage remains for spacing override.The rule uses
!importantto ensure proper spacing when the show-all button is expanded. This was noted in a previous review; thestylelint-disable-lineindicates this is an accepted trade-off.src/app/examples/si-application-header/si-launchpad.ts (2)
122-162: LGTM!The computed
appsproperty correctly derives either a flatApp[]or categorizedAppCategory[]based on theenableCategoriessignal. The categorization logic with hardcoded name matching is acceptable for demonstration code.A previous review suggested adding a category field for more maintainable categorization, which remains a valid optional improvement if this pattern were used in production.
164-177: Direct mutation ofapp.favoritebefore triggering reactivity.The
updateFavoritemethod mutatesapp.favoritedirectly before callingset(). While functional, this can cause subtle issues if the same app object is referenced elsewhere.A previous review suggested an immutable update pattern using
map()to create new objects. This remains a valid optional improvement.The
toggleFavoritesandtoggleCategoriesmethods correctly cast the event target and update signals.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (12)
playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (29)
api-goldens/element-ng/application-header/index.api.mdapi-goldens/element-ng/translate/index.api.mdplaywright/e2e/element-examples/si-launchpad.spec.tsplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tsprojects/element-ng/application-header/launchpad/si-launchpad.model.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.htmlprojects/element-ng/application-header/si-application-header.component.scssprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/translate/si-translatable-keys.interface.tsprojects/element-theme/src/styles/components/_application-header.scssprojects/element-theme/src/styles/variables/_zindex.scsssrc/app/examples/si-application-header/si-launchpad.htmlsrc/app/examples/si-application-header/si-launchpad.ts
💤 Files with no reviewable changes (2)
- api-goldens/element-ng/translate/index.api.md
- projects/element-ng/translate/si-translatable-keys.interface.ts
🧰 Additional context used
🧠 Learnings (28)
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
api-goldens/element-ng/application-header/index.api.mdprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.ts
📚 Learning: 2025-12-23T09:24:35.163Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/translate/index.api.md:12-84
Timestamp: 2025-12-23T09:24:35.163Z
Learning: In the siemens/element repository, do not review files under the api-goldens/ directory (e.g., api-goldens/**/index.api.md) since they are auto-generated by API Extractor. Exclude these from review checks and focus on source files that are not auto-generated.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-23T09:24:57.843Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/index.api.md:324-325
Timestamp: 2025-12-23T09:24:57.843Z
Learning: Do not review files in the api-goldens directory, as they are auto-generated API reports produced by API Extractor.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-23T09:24:40.395Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/module-federation/index.api.md:7-11
Timestamp: 2025-12-23T09:24:40.395Z
Learning: In the siemens/element repository, do not review any files under the api-goldens directory, as they are auto-generated API reports produced by API Extractor. These MD files should be excluded from review unless explicitly overridden.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-15T10:05:59.100Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1193
File: projects/element-ng/search-bar/si-search-bar.component.spec.ts:21-28
Timestamp: 2025-12-15T10:05:59.100Z
Learning: In tests that rely on Jasmine's fake clock to control time-based RxJS operators (e.g., debounceTime), use jasmine.clock().install() together with jasmine.clock().mockDate() to enable precise timing control. Without mockDate, scheduler-based operators may not respond to the fake clock. Apply this guidance to spec files (e.g., any file named *.spec.ts or *.spec.js) that test time-sensitive behavior, ensuring you install and mockDate before advancing time in tests.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad.spec.tsplaywright/e2e/element-examples/si-launchpad.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsplaywright/e2e/element-examples/si-launchpad.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/launchpad/si-launchpad.model.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tsprojects/element-ng/application-header/si-application-header.component.spec.tssrc/app/examples/si-application-header/si-launchpad.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.ts
📚 Learning: 2025-11-26T13:50:02.735Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1090
File: projects/element-ng/typeahead/si-typeahead.directive.ts:511-515
Timestamp: 2025-11-26T13:50:02.735Z
Learning: In the typeahead component (projects/element-ng/typeahead/si-typeahead.directive.ts), the `createOption()` method should always close the dropdown with `this.canBeOpen.set(false)` regardless of whether `typeaheadMultiSelect` is enabled. This is different from `selectMatch()` which only closes the dropdown when NOT in multi-select mode.
Applied to files:
projects/element-ng/application-header/si-application-header.component.htmlprojects/element-ng/application-header/si-application-header.component.ts
📚 Learning: 2025-12-09T14:32:34.036Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:21-47
Timestamp: 2025-12-09T14:32:34.036Z
Learning: In projects/element-ng/chat-messages, nested if blocks are preferred over switch statements for branching on message types in Angular templates.
Applied to files:
projects/element-ng/application-header/si-application-header.component.html
📚 Learning: 2025-12-16T16:19:17.950Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1207
File: playwright/snapshots/si-filtered-search.spec.ts-snapshots/si-filtered-search--si-filtered-search-playground--data-entered.yaml:38-38
Timestamp: 2025-12-16T16:19:17.950Z
Learning: Do not review files in the playwright/snapshots directory, as they contain auto-generated test artifacts. Exclude any YAML snapshot files under playwright/snapshots from code reviews in the siemens/element repository. If a YAML file is not an auto-generated snapshot, verify its origin before skipping.
Applied to files:
playwright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yaml
📚 Learning: 2025-12-11T10:09:01.564Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1151
File: projects/element-ng/auto-collapsable-list/si-auto-collapsable-list.directive.spec.ts:83-104
Timestamp: 2025-12-11T10:09:01.564Z
Learning: In the auto-collapsable-list directive tests (projects/element-ng/auto-collapsable-list/si-auto-collapsable-list.directive.spec.ts), jasmine.clock() successfully controls the directive's setTimeout-based layout logic in zoneless mode. Mock timers work fine here, unlike in select component overflow tests where real setTimeout waits are required.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T11:54:31.132Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1133
File: projects/element-ng/tour/si-tour.service.spec.ts:21-26
Timestamp: 2025-12-04T11:54:31.132Z
Learning: In the siemens/element repository, all components are standalone by default and do not require the explicit `standalone: true` flag. Components should be added to the `imports` array in TestBed configuration, not the `declarations` array.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tssrc/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2025-12-08T11:24:45.272Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/options/si-select-lazy-options.directive.spec.ts:67-85
Timestamp: 2025-12-08T11:24:45.272Z
Learning: In select lazy options tests (projects/element-ng/select/options/si-select-lazy-options.directive.spec.ts), jasmine.clock() cannot control RxJS debounceTime in zoneless mode. Real setTimeout waits must be used instead for tests involving search debouncing.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-08T11:25:20.861Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/si-select.component.spec.ts:204-205
Timestamp: 2025-12-08T11:25:20.861Z
Learning: In zoneless Angular tests (using provideZonelessChangeDetection), calling fixture.changeDetectorRef.markForCheck() before fixture.detectChanges() is required even for components using the default change detection strategy (not OnPush). This differs from zone.js-based testing where markForCheck is primarily needed only for OnPush components.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T05:50:17.637Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1135
File: projects/element-ng/tree-view/si-tree-view.component.spec.ts:953-964
Timestamp: 2025-12-04T05:50:17.637Z
Learning: In the siemens/element repository, when making tree-view tests zoneless-ready, jasmine.clock() should be used to mimic setTimeouts even in zoneless mode, as the component implementation relies on setTimeout calls that need to be controlled in tests.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T07:16:53.762Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts:92-105
Timestamp: 2025-12-15T07:16:53.762Z
Learning: In gridstack-wrapper component tests (projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts), real setTimeout waits must be used instead of fixture.whenStable() to avoid injector destroyed errors during mount/unmount operations in zoneless mode. The GridStack timing-dependent operations require actual async delays.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T05:50:38.735Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1135
File: projects/element-ng/tree-view/si-tree-view.component.spec.ts:1244-1245
Timestamp: 2025-12-04T05:50:38.735Z
Learning: In zoneless Angular tests with OnPush components, calling `await runOnPushChangeDetection(fixture)` followed by `await fixture.whenStable()` is NOT redundant. The `runOnPushChangeDetection` helper forces change detection to run for OnPush components, while `whenStable()` waits for all async operations to complete. Both are necessary and serve different purposes.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-02T06:50:25.778Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1104
File: projects/element-ng/datepicker/components/si-day-selection.component.spec.ts:19-19
Timestamp: 2025-12-02T06:50:25.778Z
Learning: The `runOnPushChangeDetection` helper is located at `projects/element-ng/test-helpers/change-detection.helper.ts` and is shared across the element-ng project, not scoped under individual feature modules like datepicker.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.ts
📚 Learning: 2025-12-01T04:01:27.365Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1099
File: projects/element-ng/accordion/si-accordion.component.spec.ts:39-43
Timestamp: 2025-12-01T04:01:27.365Z
Learning: With Angular 20 and esbuild, synchronous `TestBed.configureTestingModule(...).compileComponents()` calls work correctly even when components use external templates (templateUrl), because esbuild pre-compiles templates at build time. The async/await pattern is not required in this scenario.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-01T14:12:11.111Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1104
File: projects/element-ng/datepicker/components/si-day-selection.component.spec.ts:198-202
Timestamp: 2025-12-01T14:12:11.111Z
Learning: In zoneless Angular tests (using provideZonelessChangeDetection), prefer `await fixture.whenStable()` over `fixture.detectChanges()` after dispatching events or triggering async operations, as `whenStable()` waits for all asynchronous operations to complete and better mimics the production environment.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-08T11:25:51.584Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/si-select.component.spec.ts:511-516
Timestamp: 2025-12-08T11:25:51.584Z
Learning: In the select component overflow tests (projects/element-ng/select/si-select.component.spec.ts), jasmine.clock() cannot be used to control the timing for overflow item detection. Real setTimeout waits must be used instead, likely due to ResizeObserver or debounced layout calculations that jasmine.clock() cannot control in zoneless mode.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-05T08:00:38.407Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1151
File: projects/element-ng/typeahead/si-typeahead.directive.spec.ts:114-121
Timestamp: 2025-12-05T08:00:38.407Z
Learning: In the typeahead test suite (projects/element-ng/typeahead/si-typeahead.directive.spec.ts), jasmine.clock() must be installed in beforeAll and uninstalled in afterAll (not beforeEach/afterEach) because using per-test clock installation causes schedulerFunction-related issues in these specific tests.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T07:16:32.082Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts:69-80
Timestamp: 2025-12-15T07:16:32.082Z
Learning: In gridstack-wrapper component tests (projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts), real setTimeout waits (e.g., `await new Promise(resolve => setTimeout(resolve, 0))`) must be used instead of `fixture.whenStable()` when testing grid item rendering and initialization. This is likely due to gridstack library's initialization timing or lifecycle behavior that fixture.whenStable() cannot properly wait for in zoneless mode.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-22T13:04:20.883Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: src/app/examples/ag-grid/ag-grid-empty-state.ts:20-24
Timestamp: 2025-12-22T13:04:20.883Z
Learning: The SiEmptyStateComponent in siemens/element-ng accepts icon names in kebab-case format (e.g., 'element-technical-operator') that may not be directly exported as constants in the element-icons.ts file. Do not flag these as errors if they render correctly at runtime.
Applied to files:
projects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tssrc/app/examples/si-application-header/si-launchpad.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.ts
📚 Learning: 2025-12-22T13:29:31.048Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/ag-grid/parts/color-scheme.part.ts:35-35
Timestamp: 2025-12-22T13:29:31.048Z
Learning: In the Element design system (projects/element-ng/theme and projects/element-theme), only `--element-body-font-family` is exposed as a CSS custom property for typography. Font sizes are NOT exposed as CSS custom properties; they are defined using internal SCSS variables (e.g., `typography.$si-font-size-body`) that compile to fixed values. Using hardcoded pixel values for fontSize in AG Grid theme configuration (e.g., `fontSize: '14px'`) is the correct approach.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-17T14:01:05.705Z
Learnt from: dauriamarco
Repo: siemens/element PR: 908
File: projects/element-ng/side-panel/si-side-panel.component.ts:70-74
Timestamp: 2025-12-17T14:01:05.705Z
Learning: In the Element side-panel component (projects/element-ng/side-panel/si-side-panel.component.ts), the `xxxlMinimum` breakpoint (1920px) is intentionally hardcoded as a private static constant rather than being part of the configurable `Breakpoints` interface. This design preserves full Bootstrap compatibility (which only defines breakpoints up to xxl/1400px) while allowing Element to handle ultra-wide screens explicitly without affecting the standard breakpoint configuration.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-03T13:02:34.875Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1058
File: docs/fundamentals/layouts/breakpoints.md:25-31
Timestamp: 2025-12-03T13:02:34.875Z
Learning: In Element documentation for CSS breakpoints and class infixes (e.g., in docs/fundamentals/layouts/breakpoints.md), _None_ is formatted with italics to indicate the absence of a class infix (representing base mobile-first styles), while actual CSS class infixes like `sm`, `md`, `lg`, `xl`, `xxl` use inline code formatting with backticks. This semantic formatting distinction is intentional.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-15T07:17:06.981Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/widget-instance-editor-dialog/si-widget-instance-editor-dialog.component.spec.ts:37-49
Timestamp: 2025-12-15T07:17:06.981Z
Learning: In widget-instance-editor-dialog component tests (projects/dashboards-ng/src/components/widget-instance-editor-dialog/si-widget-instance-editor-dialog.component.spec.ts), real setTimeout waits (e.g., `await new Promise(resolve => setTimeout(resolve, 0))`) must be used instead of `fixture.whenStable()` when testing component initialization. This is likely due to the component's lifecycle behavior or modal initialization timing that fixture.whenStable() cannot properly wait for in zoneless mode.
Applied to files:
projects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-09T14:33:54.441Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: src/app/examples/si-chat-messages/si-ai-message.ts:24-43
Timestamp: 2025-12-09T14:33:54.441Z
Learning: In src/app/examples/ (example/demo code) directories, allow and expect code duplication. Treat duplication as acceptable for demonstration code that aims to illustrate usage rather than enforce DRY. Do not enforce refactoring to remove duplication in example files; focus on readability and clarity of examples. This applies to all TypeScript files under src/app/examples (including nested subdirectories like si-chat-messages).
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
🧬 Code graph analysis (2)
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (1)
projects/element-ng/common/helpers/rtl.ts (1)
correctKeyRTL(9-14)
src/app/examples/si-application-header/si-launchpad.ts (1)
projects/element-ng/application-header/launchpad/si-launchpad.model.ts (2)
App(55-55)AppCategory(31-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (39)
projects/element-theme/src/styles/components/_application-header.scss (1)
34-38: Previous feedback addressed - comment now matches implementation.The comment has been updated to accurately reflect the current CSS implementation. The description now correctly states that the border is hidden when "navigation is expanded" rather than referencing the outdated "header-toggle" term, aligning with the
show-navigationclass used in the selector.projects/element-ng/application-header/testing/si-launchpad.harness.ts (1)
19-21: LGTM! Category name updated to reflect UI changes.The favorite category name change from "Favorite apps" to "Favorites" correctly aligns with the broader UI text updates mentioned in the PR objectives.
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.ts (2)
15-15: LGTM! Animation provider import added.
51-56: LGTM! Disabling animations for test stability.Adding
provideNoopAnimations()is the correct approach to prevent animation timing issues in zoneless tests, consistent with similar changes across the test suite.projects/element-theme/src/styles/variables/_zindex.scss (1)
19-20: LGTM! Z-index layering adjusted for proper stacking.The launchpad now stacks one level below the application header (
$zindex-application-header - 1), ensuring the header remains above the launchpad in the visual hierarchy. This creates the intended layering: header (top) → launchpad → backdrop.projects/element-ng/application-header/si-application-header.component.spec.ts (5)
15-15: LGTM! Animation provider import added.
106-106: LGTM! Animations disabled for test stability.
221-223: LGTM! Animations disabled for test stability.
309-312: LGTM! Animations disabled for test stability.
360-362: LGTM! Animations disabled for test stability.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss (5)
3-3: LGTM! Import updated to all-variables.The import change aligns with the modernization of variable references throughout the stylesheet.
8-11: LGTM! Inline comment added for 1px offset.The added comment
/* header border compensation */clearly explains the intentional 1px offset, addressing the previous review feedback.
18-26: LGTM! Responsive padding improvements.The padding refactor uses the spacer map consistently and adds responsive behavior for medium breakpoints, improving the launchpad UX on smaller screens.
29-33: LGTM! New header layout structure added.The
.apps-headerblock provides proper flexbox layout for the launchpad header elements.
6-7: The positioning implementation is correct. Theposition: absoluteon.app-switcherwith parent.launchpad-containerasposition: fixedis the proper pattern for a modal overlay. The component correctly handles scroll states viaoverflow-y: autoon.apps-scroll, maintains responsive behavior through media queries (media-breakpoint-down(md)), and uses viewport-relative sizing (calc(100vh - ...)) to ensure proper display across all breakpoints. Thepointer-eventsdelegation (none on parent, all on child) properly enables interaction on the launchpad content.projects/element-ng/application-header/si-application-header.component.scss (2)
5-6: LGTM!Setting the initial opacity to 0 allows the Angular animation trigger to control the backdrop's fade-in/fade-out transitions.
8-13: Verify that launchpad content is interactive.The
pointer-events: noneon the container prevents the overlay from blocking clicks, but child elements needpointer-events: autoto remain interactive. Ensure the launchpad content inside this container has pointer-events re-enabled.projects/element-ng/application-header/launchpad/si-launchpad.spec.ts (1)
115-115: LGTM!The test expectation correctly reflects the updated category label from "Favorite apps" to "Favorites", aligning with the UI text changes in this PR.
projects/element-ng/application-header/launchpad/si-launchpad.model.ts (1)
51-52: LGTM!Adding the optional
disabledproperty toAppBaseis backward-compatible and enables disabling apps in the launchpad. The property is properly inherited byAppLinkandAppRouterLinkviaAppBase.playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yaml (1)
1-25: Auto-generated snapshot file.This file contains auto-generated test artifacts that reflect the UI changes in the PR. No manual review required.
playwright/e2e/element-examples/si-launchpad.spec.ts (1)
16-16: LGTM!The test now marks "Fischbach" as the favorite item instead of "Rocket", which aligns with the updated launchpad test scenarios in this PR.
playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yaml (1)
1-25: Auto-generated snapshot file.This file contains auto-generated test artifacts that reflect the UI changes in the PR. No manual review required.
projects/element-ng/application-header/si-application-header.component.ts (3)
5-11: LGTM!Importing Angular animation utilities and aliasing
triggerasanimationTriggerto avoid naming conflicts with RxJS is a good practice.
51-80: LGTM! Animation definitions are well-structured.The animation triggers are properly defined:
expand: Handles the launchpad container's entrance (translateY(-120px)→0) and exit with appropriate timings (0.5s enter, 0.25s leave).backdrop: Controls opacity transitions with a fast linear animation (0.15s).
183-189: Add breakpoint monitoring to openLaunchpad() or verify template handles responsive state correctly.The launchpad uses a signal-based approach without monitoring breakpoint changes while open. When launchpad is opened in mobile view and the viewport expands to desktop,
launchpadOpensignal remains true even though the button is hidden by CSS. OnlyopenMobileNavigation()subscribes toinlineDropdownchanges. Either:
- Add a subscription to
inlineDropdowninopenLaunchpad()similar toopenMobileNavigation()to close launchpad on breakpoint changes, or- Explicitly test and document that CSS-driven visibility is the intended responsive behavior
The existing tests use
expandBreakpoint="never"and don't cover the scenario of launchpad remaining open during a responsive breakpoint transition.projects/element-ng/application-header/si-application-header.component.html (2)
56-60: LGTM!The launchpad container wrapper with the
[@expand]animation trigger properly enables the entrance/exit animations defined in the component. The template outlet and injector binding are correctly preserved.
67-67: Animation binding is simplified and correct.The backdrop animation binding uses a static
'show'value, which is appropriate since the element only exists when visible (controlled by the@ifcondition). This approach is clean and the* <=> *transition in the animation definition handles the void state transitions correctly.projects/element-ng/application-header/launchpad/si-launchpad-app.component.ts (1)
34-36: LGTM! Disabled state implementation is correct.The disabled functionality is properly implemented with appropriate host bindings to remove the element from keyboard navigation (
tabindex="-1"), visual feedback (.disabledclass), and early-exit guards in both event handlers to prevent interaction. This follows accessibility best practices for disabled interactive elements.Also applies to: 50-51, 61-62, 68-69
projects/element-ng/application-header/launchpad/si-launchpad-app.component.html (1)
6-18: LGTM! Template restructuring supports responsive design.The new
.app-textcontainer with proper heading hierarchy and text truncation classes provides a clean, semantic structure that aligns with the responsive design goals. The conditional external icon rendering and proper use of flexbox utilities ensure the layout adapts correctly across breakpoints.projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss (1)
2-14: LGTM! Styling updates align with design system tokens.The migration to
all-variablestokens and the responsive layout implementation usingmedia-breakpoint-down(md)follow the Element design system conventions. The disabled state properly uses the design token for opacity, and the responsive rules appropriately adjust layout, spacing, and positioning for mobile viewports.Also applies to: 27-30, 75-100
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html (1)
10-25: LGTM! Template updates properly implement disabled state and keyboard navigation.The restructured header with the close button improves accessibility and UX. The
[disabled]bindings on launchpad app items (lines 49, 76) correctly coerce the optional property with!!app.disabled, and the(keydown)handlers (lines 59, 79) consistently enable keyboard navigation for both router-link and anchor variants.Also applies to: 49-49, 59-59, 76-76, 79-79
src/app/examples/si-application-header/si-launchpad.html (1)
31-38: All signal implementations are properly defined and used in the component:
enableFavoritesandenableCategoriesare declared assignal()appsis a computed signal with reactive transformation that depends onenableCategories()- Event handlers
toggleFavorites()andtoggleCategories()correctly update the signals using.set()updateFavorite()properly triggers reactivity by callingbaseApps.set()src/app/examples/si-application-header/si-launchpad.ts (3)
5-14: LGTM!The imports are correctly updated to include Angular's
signalandcomputedfor reactive state management, andAppCategoryfrom the element-ng library for type safety when using categorized apps.
29-31: LGTM!The reactive signals for
enableFavoritesandenableCategoriesare well-initialized with sensible defaults that match the expected demo behavior.
32-120: LGTM!The
baseAppssignal provides comprehensive sample data demonstrating various app states: favorites, active, external, disabled, and long names. This effectively exercises the launchpad's display capabilities for demo purposes.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (4)
61-64: Title text updated to "Switch to application".This aligns with the iX style per the PR discussion. Note that there's ongoing discussion about whether "application" vs "app" terminology is appropriate for certain product contexts. Ensure this change is coordinated with stakeholders mentioned in the PR comments.
66-70: LGTM!The
subtitleTextis now optional with no default value, and the documentation clearly indicates no subtitle is displayed when not provided. This is a cleaner API design.
84-90: LGTM!The favorite apps section title is simplified from "Favorite apps" to "Favorites", which is more concise.
159-199: LGTM! Keyboard navigation logic is well-implemented.The
handleAppsNavigationmethod correctly:
- Uses
correctKeyRTLfor RTL support- Validates input before processing
- Uses
:scope >selector for proper element isolation (addressing the previous review comment)- Handles horizontal navigation with wrap-around
- Delegates vertical navigation to a dedicated method
- Calls
preventDefault()to avoid scroll conflicts
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
Outdated
Show resolved
Hide resolved
d1ec321 to
31e1484
Compare
|
Do we need the disabled state @panch1739 ? I see no reason in having one. Disabling links is not really a thing. |
@dauriamarco I love itttt, is looking much better 👯 Im not sure if you were ready for a review, but some small finding:
@spike-rabbit you are right, disabled links are not a thing...but i think we carried the disabled state from the current implementation. Is this correct or did i invented that? I would be ok with removing it, i think we didnt even defined for ix. |
|
Based on the code, it seems the disabled state is new. @dauriamarco can you please remove it please. |
31e1484 to
dba42a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@projects/element-ng/application-header/testing/si-launchpad-category.harness.ts:
- Line 18: The test harness uses a brittle style-class selector in the private
name locator (private name = this.locatorForOptional('.si-h4.mb-6')); replace or
supplement this with a stable data-test-id attribute: update the component
template to add a data-test-id (e.g., data-test-id="si-launchpad-category-name")
and change the harness to use
locatorForOptional('[data-test-id="si-launchpad-category-name"]') instead of the
`.si-h4.mb-6` selector to avoid fragility from styling changes.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (12)
playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (29)
api-goldens/element-ng/application-header/index.api.mdapi-goldens/element-ng/translate/index.api.mdplaywright/e2e/element-examples/si-launchpad.spec.tsplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.htmlprojects/element-ng/application-header/si-application-header.component.scssprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/translate/si-translatable-keys.interface.tsprojects/element-theme/src/styles/components/_application-header.scssprojects/element-theme/src/styles/variables/_zindex.scsssrc/app/examples/si-application-header/si-launchpad.htmlsrc/app/examples/si-application-header/si-launchpad.ts
💤 Files with no reviewable changes (2)
- api-goldens/element-ng/translate/index.api.md
- projects/element-ng/translate/si-translatable-keys.interface.ts
🧰 Additional context used
🧠 Learnings (34)
📚 Learning: 2025-12-11T10:09:01.564Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1151
File: projects/element-ng/auto-collapsable-list/si-auto-collapsable-list.directive.spec.ts:83-104
Timestamp: 2025-12-11T10:09:01.564Z
Learning: In the auto-collapsable-list directive tests (projects/element-ng/auto-collapsable-list/si-auto-collapsable-list.directive.spec.ts), jasmine.clock() successfully controls the directive's setTimeout-based layout logic in zoneless mode. Mock timers work fine here, unlike in select component overflow tests where real setTimeout waits are required.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T07:16:53.762Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts:92-105
Timestamp: 2025-12-15T07:16:53.762Z
Learning: In gridstack-wrapper component tests (projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts), real setTimeout waits must be used instead of fixture.whenStable() to avoid injector destroyed errors during mount/unmount operations in zoneless mode. The GridStack timing-dependent operations require actual async delays.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T05:50:17.637Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1135
File: projects/element-ng/tree-view/si-tree-view.component.spec.ts:953-964
Timestamp: 2025-12-04T05:50:17.637Z
Learning: In the siemens/element repository, when making tree-view tests zoneless-ready, jasmine.clock() should be used to mimic setTimeouts even in zoneless mode, as the component implementation relies on setTimeout calls that need to be controlled in tests.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T07:16:32.082Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts:69-80
Timestamp: 2025-12-15T07:16:32.082Z
Learning: In gridstack-wrapper component tests (projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts), real setTimeout waits (e.g., `await new Promise(resolve => setTimeout(resolve, 0))`) must be used instead of `fixture.whenStable()` when testing grid item rendering and initialization. This is likely due to gridstack library's initialization timing or lifecycle behavior that fixture.whenStable() cannot properly wait for in zoneless mode.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-08T11:25:51.584Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/si-select.component.spec.ts:511-516
Timestamp: 2025-12-08T11:25:51.584Z
Learning: In the select component overflow tests (projects/element-ng/select/si-select.component.spec.ts), jasmine.clock() cannot be used to control the timing for overflow item detection. Real setTimeout waits must be used instead, likely due to ResizeObserver or debounced layout calculations that jasmine.clock() cannot control in zoneless mode.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-08T11:24:45.272Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/options/si-select-lazy-options.directive.spec.ts:67-85
Timestamp: 2025-12-08T11:24:45.272Z
Learning: In select lazy options tests (projects/element-ng/select/options/si-select-lazy-options.directive.spec.ts), jasmine.clock() cannot control RxJS debounceTime in zoneless mode. Real setTimeout waits must be used instead for tests involving search debouncing.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-05T08:00:38.407Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1151
File: projects/element-ng/typeahead/si-typeahead.directive.spec.ts:114-121
Timestamp: 2025-12-05T08:00:38.407Z
Learning: In the typeahead test suite (projects/element-ng/typeahead/si-typeahead.directive.spec.ts), jasmine.clock() must be installed in beforeAll and uninstalled in afterAll (not beforeEach/afterEach) because using per-test clock installation causes schedulerFunction-related issues in these specific tests.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T07:17:06.981Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/widget-instance-editor-dialog/si-widget-instance-editor-dialog.component.spec.ts:37-49
Timestamp: 2025-12-15T07:17:06.981Z
Learning: In widget-instance-editor-dialog component tests (projects/dashboards-ng/src/components/widget-instance-editor-dialog/si-widget-instance-editor-dialog.component.spec.ts), real setTimeout waits (e.g., `await new Promise(resolve => setTimeout(resolve, 0))`) must be used instead of `fixture.whenStable()` when testing component initialization. This is likely due to the component's lifecycle behavior or modal initialization timing that fixture.whenStable() cannot properly wait for in zoneless mode.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.ts
📚 Learning: 2025-12-15T10:05:59.100Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1193
File: projects/element-ng/search-bar/si-search-bar.component.spec.ts:21-28
Timestamp: 2025-12-15T10:05:59.100Z
Learning: In tests that rely on Jasmine's fake clock to control time-based RxJS operators (e.g., debounceTime), use jasmine.clock().install() together with jasmine.clock().mockDate() to enable precise timing control. Without mockDate, scheduler-based operators may not respond to the fake clock. Apply this guidance to spec files (e.g., any file named *.spec.ts or *.spec.js) that test time-sensitive behavior, ensuring you install and mockDate before advancing time in tests.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsplaywright/e2e/element-examples/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.ts
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsplaywright/e2e/element-examples/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/si-application-header.component.spec.tssrc/app/examples/si-application-header/si-launchpad.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-30T13:52:33.581Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1221
File: projects/element-ng/side-panel/si-side-panel.component.ts:136-142
Timestamp: 2025-12-30T13:52:33.581Z
Learning: Maintain the existing negative naming convention (e.g., disableBackdrop) across the siemens/element repository for consistency. Do not refactor to positive names (e.g., showBackdrop, hasBackdrop) unless the project explicitly changes the convention. In reviews, flag changes that alter established naming patterns in related files under projects/element-ng.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-08T11:25:20.861Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/si-select.component.spec.ts:204-205
Timestamp: 2025-12-08T11:25:20.861Z
Learning: In zoneless Angular tests (using provideZonelessChangeDetection), calling fixture.changeDetectorRef.markForCheck() before fixture.detectChanges() is required even for components using the default change detection strategy (not OnPush). This differs from zone.js-based testing where markForCheck is primarily needed only for OnPush components.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T05:50:38.735Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1135
File: projects/element-ng/tree-view/si-tree-view.component.spec.ts:1244-1245
Timestamp: 2025-12-04T05:50:38.735Z
Learning: In zoneless Angular tests with OnPush components, calling `await runOnPushChangeDetection(fixture)` followed by `await fixture.whenStable()` is NOT redundant. The `runOnPushChangeDetection` helper forces change detection to run for OnPush components, while `whenStable()` waits for all async operations to complete. Both are necessary and serve different purposes.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-02T06:50:25.778Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1104
File: projects/element-ng/datepicker/components/si-day-selection.component.spec.ts:19-19
Timestamp: 2025-12-02T06:50:25.778Z
Learning: The `runOnPushChangeDetection` helper is located at `projects/element-ng/test-helpers/change-detection.helper.ts` and is shared across the element-ng project, not scoped under individual feature modules like datepicker.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.ts
📚 Learning: 2025-12-01T04:01:27.365Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1099
File: projects/element-ng/accordion/si-accordion.component.spec.ts:39-43
Timestamp: 2025-12-01T04:01:27.365Z
Learning: With Angular 20 and esbuild, synchronous `TestBed.configureTestingModule(...).compileComponents()` calls work correctly even when components use external templates (templateUrl), because esbuild pre-compiles templates at build time. The async/await pattern is not required in this scenario.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-01T14:12:11.111Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1104
File: projects/element-ng/datepicker/components/si-day-selection.component.spec.ts:198-202
Timestamp: 2025-12-01T14:12:11.111Z
Learning: In zoneless Angular tests (using provideZonelessChangeDetection), prefer `await fixture.whenStable()` over `fixture.detectChanges()` after dispatching events or triggering async operations, as `whenStable()` waits for all asynchronous operations to complete and better mimics the production environment.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-16T16:19:17.950Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1207
File: playwright/snapshots/si-filtered-search.spec.ts-snapshots/si-filtered-search--si-filtered-search-playground--data-entered.yaml:38-38
Timestamp: 2025-12-16T16:19:17.950Z
Learning: Do not review files in the playwright/snapshots directory, as they contain auto-generated test artifacts. Exclude any YAML snapshot files under playwright/snapshots from code reviews in the siemens/element repository. If a YAML file is not an auto-generated snapshot, verify its origin before skipping.
Applied to files:
playwright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yaml
📚 Learning: 2025-11-26T13:50:02.735Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1090
File: projects/element-ng/typeahead/si-typeahead.directive.ts:511-515
Timestamp: 2025-11-26T13:50:02.735Z
Learning: In the typeahead component (projects/element-ng/typeahead/si-typeahead.directive.ts), the `createOption()` method should always close the dropdown with `this.canBeOpen.set(false)` regardless of whether `typeaheadMultiSelect` is enabled. This is different from `selectMatch()` which only closes the dropdown when NOT in multi-select mode.
Applied to files:
projects/element-ng/application-header/si-application-header.component.htmlprojects/element-ng/application-header/si-application-header.component.ts
📚 Learning: 2025-12-30T13:52:41.363Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1221
File: projects/element-ng/side-panel/si-side-panel.component.ts:136-142
Timestamp: 2025-12-30T13:52:41.363Z
Learning: In the siemens/element repository, the library uses negative naming conventions like `disableBackdrop` intentionally for consistency across the codebase. This pattern should be maintained rather than converting to positive naming conventions like `showBackdrop` or `hasBackdrop`.
Applied to files:
projects/element-ng/application-header/si-application-header.component.html
📚 Learning: 2025-12-09T14:32:34.036Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:21-47
Timestamp: 2025-12-09T14:32:34.036Z
Learning: In projects/element-ng/chat-messages, nested if blocks are preferred over switch statements for branching on message types in Angular templates.
Applied to files:
projects/element-ng/application-header/si-application-header.component.html
📚 Learning: 2025-12-22T13:04:35.578Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: src/app/examples/ag-grid/ag-grid-empty-state.ts:20-24
Timestamp: 2025-12-22T13:04:35.578Z
Learning: The SiEmptyStateComponent in siemens/element-ng accepts icon names in kebab-case format (e.g., 'element-technical-operator') that may not be directly exported as constants in the element-icons.ts file. Do not flag these as errors if they render correctly at runtime.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsapi-goldens/element-ng/application-header/index.api.mdprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scsssrc/app/examples/si-application-header/si-launchpad.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-23T09:24:35.163Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/translate/index.api.md:12-84
Timestamp: 2025-12-23T09:24:35.163Z
Learning: In the siemens/element repository, do not review files under the api-goldens/ directory (e.g., api-goldens/**/index.api.md) since they are auto-generated by API Extractor. Exclude these from review checks and focus on source files that are not auto-generated.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-23T09:24:57.843Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/index.api.md:324-325
Timestamp: 2025-12-23T09:24:57.843Z
Learning: Do not review files in the api-goldens directory, as they are auto-generated API reports produced by API Extractor.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-23T09:24:40.395Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/module-federation/index.api.md:7-11
Timestamp: 2025-12-23T09:24:40.395Z
Learning: In the siemens/element repository, do not review any files under the api-goldens directory, as they are auto-generated API reports produced by API Extractor. These MD files should be excluded from review unless explicitly overridden.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-04T11:54:31.132Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1133
File: projects/element-ng/tour/si-tour.service.spec.ts:21-26
Timestamp: 2025-12-04T11:54:31.132Z
Learning: In the siemens/element repository, all components are standalone by default and do not require the explicit `standalone: true` flag. Components should be added to the `imports` array in TestBed configuration, not the `declarations` array.
Applied to files:
projects/element-ng/application-header/si-application-header.component.spec.tssrc/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2026-01-02T06:41:22.404Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: playwright/e2e/element-examples/static.spec.ts:119-121
Timestamp: 2026-01-02T06:41:22.404Z
Learning: In the Element design system (siemens/element repository), disabling the `aria-required-children` accessibility rule for `SiEmptyStateComponent` is an accepted pattern. This component is used in AG Grid overlays (AgNoRowsOverlayComponent) and other places in Element, and the rule is consistently disabled in tests where this component appears (e.g., si-pills-input, datatable-row-group, ag-grid-empty-state).
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.ts
📚 Learning: 2026-01-02T16:41:57.274Z
Learnt from: kfenner
Repo: siemens/element PR: 1005
File: docs/fundamentals/styles/sizing.md:18-26
Timestamp: 2026-01-02T16:41:57.274Z
Learning: In Element documentation (siemens/element), sizing utility examples in docs may intentionally differ from implementation examples to demonstrate multiple valid approaches - for example, documentation might use traditional CSS properties like `height` and `width` while the implementation example uses logical properties like `block-size` and `inline-size`. These are considered different examples rather than inconsistencies.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-22T13:29:36.922Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/ag-grid/parts/color-scheme.part.ts:35-35
Timestamp: 2025-12-22T13:29:36.922Z
Learning: In the Element design system (projects/element-ng/theme and projects/element-theme), only `--element-body-font-family` is exposed as a CSS custom property for typography. Font sizes are NOT exposed as CSS custom properties; they are defined using internal SCSS variables (e.g., `typography.$si-font-size-body`) that compile to fixed values. Using hardcoded pixel values for fontSize in AG Grid theme configuration (e.g., `fontSize: '14px'`) is the correct approach.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-31T07:16:13.451Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/ag-grid/parts/radio-style.part.ts:15-61
Timestamp: 2025-12-31T07:16:13.451Z
Learning: In the AG Grid theme package (projects/element-ng/ag-grid), styling should maintain consistency with the base Element design system components rather than adding enhancements or extra CSS. If accessibility improvements like focus-visible indicators are needed, they should be implemented at the Element design system level first.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-17T14:01:05.705Z
Learnt from: dauriamarco
Repo: siemens/element PR: 908
File: projects/element-ng/side-panel/si-side-panel.component.ts:70-74
Timestamp: 2025-12-17T14:01:05.705Z
Learning: In the Element side-panel component (projects/element-ng/side-panel/si-side-panel.component.ts), the `xxxlMinimum` breakpoint (1920px) is intentionally hardcoded as a private static constant rather than being part of the configurable `Breakpoints` interface. This design preserves full Bootstrap compatibility (which only defines breakpoints up to xxl/1400px) while allowing Element to handle ultra-wide screens explicitly without affecting the standard breakpoint configuration.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-03T13:02:34.875Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1058
File: docs/fundamentals/layouts/breakpoints.md:25-31
Timestamp: 2025-12-03T13:02:34.875Z
Learning: In Element documentation for CSS breakpoints and class infixes (e.g., in docs/fundamentals/layouts/breakpoints.md), _None_ is formatted with italics to indicate the absence of a class infix (representing base mobile-first styles), while actual CSS class infixes like `sm`, `md`, `lg`, `xl`, `xxl` use inline code formatting with backticks. This semantic formatting distinction is intentional.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-09T14:33:54.441Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: src/app/examples/si-chat-messages/si-ai-message.ts:24-43
Timestamp: 2025-12-09T14:33:54.441Z
Learning: In src/app/examples/ (example/demo code) directories, allow and expect code duplication. Treat duplication as acceptable for demonstration code that aims to illustrate usage rather than enforce DRY. Do not enforce refactoring to remove duplication in example files; focus on readability and clarity of examples. This applies to all TypeScript files under src/app/examples (including nested subdirectories like si-chat-messages).
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2026-01-07T08:57:45.608Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1226
File: src/app/examples/si-navbar-vertical/si-navbar-vertical.ts:112-115
Timestamp: 2026-01-07T08:57:45.608Z
Learning: Do not strictly enforce badge color consistency in example files under src/app/examples. Treat badge color checks as non-blocking for TypeScript files in this directory; reviewers may still note inconsistencies, but they should not require changes for these example files. Apply this guidance broadly to all files within src/app/examples.
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
🧬 Code graph analysis (3)
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.ts (1)
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (1)
Component(30-277)
src/app/examples/si-application-header/si-launchpad.ts (1)
projects/element-ng/application-header/launchpad/si-launchpad.model.ts (2)
App(53-53)AppCategory(31-36)
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (2)
projects/element-translate-ng/translate/t.ts (1)
t(38-51)projects/element-ng/common/helpers/rtl.ts (1)
correctKeyRTL(9-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (40)
playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yaml (1)
1-23: Skipping review: auto-generated snapshot file.Based on learnings, this file contains auto-generated Playwright test artifacts and should not be reviewed. Snapshot files are automatically updated to reflect UI changes and are verified through automated testing.
src/app/examples/si-application-header/si-launchpad.ts (4)
5-27: LGTM! Clean signal-based reactive setup.The imports and component configuration correctly set up signal-based reactive state management with OnPush change detection, which is the recommended pattern for Angular's reactive primitives.
29-30: LGTM! Signals properly declared with sensible defaults.The signal declarations correctly expose reactive state with appropriate defaults (favorites enabled, categories disabled).
32-119: LGTM! Demo data appropriately covers various UI states.The baseApps signal provides comprehensive sample data that exercises different app configurations (favorites, external links, active state, long names), which effectively demonstrates the launchpad's capabilities.
168-176: LGTM! Toggle methods correctly handle checkbox events.Both
toggleFavoritesandtoggleCategoriesproperly cast the event target, extract the checked state, and update their respective signals. The implementation is clean and type-safe.src/app/examples/si-application-header/si-launchpad.html (3)
1-7: LGTM! Header structure is correctly configured.The header wrapper and launchpad template reference binding are properly set up with appropriate Bootstrap classes and Angular directives.
8-79: LGTM! Configuration panel is well-structured and accessible.The configuration UI properly implements:
- Accessible form controls with correctly associated labels (
for/idattributes)- ARIA semantics with
role="switch"on checkbox inputs- Correct signal accessor calls with
()for reactive bindings- Responsive Bootstrap grid layout
- Clear visual hierarchy with descriptive helper text
81-87: LGTM! Template bindings correctly use signal accessors.The launchpad factory bindings properly invoke signal accessors with
()for bothapps()andenableFavorites(), ensuring reactive updates propagate correctly to the child component.projects/element-theme/src/styles/components/_application-header.scss (1)
34-38: Comment and implementation are properly aligned.The updated comment accurately describes the CSS selectors used to hide the bottom border when either the dropdown menu is shown (
:has(.dropdown-menu.show)) or navigation is expanded (&.show-navigation).projects/element-ng/application-header/si-application-header.component.scss (2)
8-13: LGTM! Launchpad container setup looks correct.The fixed positioning with
pointer-events: noneis appropriate for a modal-like overlay container. Child elements will enable pointer events as needed.
3-6: Animation control is properly implemented.The backdrop animation trigger is defined in the component with a 'show' state that sets
opacity: 1. The template applies[@backdrop]="'show'"when the backdrop is rendered, and the transition smoothly animates opacity over 0.15s. The CSSopacity: 0serves as the base state before the animation is applied. Visibility is correctly controlled through the conditional rendering (@if (openDropdownCount() || launchpadOpen())) combined with the animation state.projects/element-theme/src/styles/variables/_zindex.scss (1)
19-20: Verify the updated z-index stacking order.The launchpad now stacks one level below the application header (instead of at the same level). This creates the hierarchy:
header (1033) > launchpad (1032) > backdrop (1031). Ensure this new stacking order doesn't cause visual issues where the header overlaps or obscures the launchpad UI.Verify visually that the launchpad is correctly positioned relative to the header when open, and that the Close button and other interactive elements remain accessible.
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.ts (1)
15-15: LGTM! Good addition of animation disabling for tests.Adding
provideNoopAnimations()ensures deterministic test behavior by disabling animations. This is a standard best practice and aligns with similar updates across the PR's test files.Also applies to: 51-56
projects/element-ng/application-header/testing/si-launchpad.harness.ts (1)
19-21: LGTM! Harness updated to match UI text change.The update from "Favorite apps" to "Favorites" correctly aligns the test harness with the UI label change introduced in this PR.
projects/element-ng/application-header/si-application-header.component.spec.ts (1)
15-15: LGTM! Proper test setup for animations.The addition of
provideNoopAnimations()across all test suites correctly disables animations during testing, which prevents timing-related issues and makes tests more deterministic. This aligns with the animation triggers added to the component itself.Also applies to: 106-106, 222-222, 310-311, 361-361
projects/element-ng/application-header/si-application-header.component.ts (1)
5-11: LGTM! Well-structured animation definitions.The animation triggers are properly configured:
- The
expandanimation provides smooth enter/exit transitions with appropriate timing (0.5s enter, 0.25s leave), following common UX patterns where exit animations are faster.- The
backdropanimation uses linear easing for opacity transitions, which is the standard approach for overlay fades.The animation values and easing functions are sensible for the launchpad UI interactions.
Also applies to: 51-80
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss (3)
3-3: LGTM! Theme variable migration and border compensation.The migration from
variablestoall-variablesis consistent throughout the file. The inline comment explaining the+1pxheader border compensation addresses previous feedback and improves maintainability.Also applies to: 9-10, 18-19, 22-22
6-7: Verify the positioning change from fixed to absolute.The launchpad positioning has changed from
position: fixedtoposition: absolute. This is a significant behavioral change that will affect how the launchpad is positioned relative to its parent container rather than the viewport.Given the PR is labeled with
breaking-changes, this appears intentional for the responsive redesign. However, ensure this change has been tested across different viewport sizes and scrolling scenarios.
29-33: LGTM! New header layout structure.The
.apps-headerflexbox layout properly supports the restructured HTML template with the header content and close button, aligning with the responsive design goals.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (4)
61-64: Text change aligns with iX design system.The default title text change from "Launchpad" to "Switch to application" aligns with the PR objectives to match the iX design system with Element v49. This breaking change is intentional and discussed in the PR comments.
68-70: Breaking API change: subtitleText is now optional with no default.The
subtitleTextinput signature changed from having a defaultTranslatableStringvalue to an optionalstringwith no default. This is a breaking change for consumers who relied on the default subtitle text. Verify that this breaking change is documented in migration notes or release documentation.
87-90: Text change aligns with design updates.The favorite apps section label shortened from "Favorite apps" to "Favorites" for a more concise UI, consistent with the broader design alignment in this PR.
143-145: Excellent keyboard navigation implementation with RTL support.The keyboard navigation implementation is well-designed:
- RTL support: Uses
correctKeyRTLto handle directional keys correctly in right-to-left layouts- Performance: Caches bounding rects in
getVerticalTargetIndexto minimize DOM access- Layout detection: Distinguishes between single-column (mobile) and grid layouts using alignment tolerance
- Edge wrapping: Properly wraps navigation at edges in both horizontal and vertical directions
- Spatial awareness: In grid layouts, finds the horizontally closest app in the target row
The selector on line 178 was simplified to
:scope > a[si-launchpad-app], which aligns with the PR discussion about removing the disabled state (per spike-rabbit's feedback).Also applies to: 163-276
projects/element-ng/application-header/launchpad/si-launchpad.spec.ts (1)
115-115: Test correctly updated to match new default text.The expectation aligns with the
favoriteAppsTextdefault value change from "Favorite apps" to "Favorites" in the factory component.playwright/e2e/element-examples/si-launchpad.spec.ts (1)
16-16: Verify the test target change is intentional.The test now clicks the favorite icon on "Fischbach" instead of "Rocket". Ensure this change reflects an intentional update to the test data or app list, and isn't masking a selection issue. The use of
.first()suggests there may be multiple elements with the name "Fischbach" - verify this is the intended behavior.projects/element-ng/application-header/si-application-header.component.html (1)
56-60: Animation triggers properly integrated.The launchpad container now has an
[@expand]animation trigger, and the backdrop has a simplified[@backdrop]="'show'"trigger. These changes align with the new animation definitions in the component TypeScript file and provide smooth transition effects for the launchpad UI.Also applies to: 67-67
projects/element-ng/application-header/launchpad/si-launchpad-app.component.html (1)
6-17: Verify spacing between app name and system name matches design feedback.PR comments from panch1739 requested 4px (
spacing-2) between the App name and System name. The current implementation usesmt-2on line 15 for the system name container. Please verify this maps to the correct 4px spacing value, as Bootstrap/Element spacing utilities may not directly correspond to the requested pixel values.playwright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yaml (1)
1-51: Skipping review of auto-generated snapshot file.Based on learnings, files in
playwright/snapshotsdirectory are auto-generated test artifacts and should not be reviewed.projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.ts (5)
21-32: LGTM!Test setup correctly configures zoneless change detection and provides a minimal mock for
SiApplicationHeaderComponent. The approach of accessing the component instance viadebugElement.children[0].componentInstanceis appropriate for testing the child component.
34-45: LGTM!The
createMockAppshelper efficiently creates mock elements with spiedgetBoundingClientRect. The partialDOMRectis sufficient since the component only usestopandleftproperties.
47-71: LGTM!Single column tests comprehensively cover sequential navigation including wrap-around behavior in both directions.
73-109: LGTM!Grid layout tests thoroughly validate spatial navigation including wrap-around between rows. The test case for single-row behavior (lines 99-108) correctly expects no movement.
111-133: LGTM!Tolerance tests properly validate the boundary conditions for
leftTolerance(20px) androwTolerance(10px) constants defined in the component.projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss (3)
20-32: LGTM!The active state correctly uses
element-base-1-selectedand hover useselement-base-1-hover, which aligns with the design feedback from panch1739.
77-102: LGTM!Responsive styles properly adapt the component layout for smaller screens. The use of
:has(.favorite-icon)for conditional padding is a clean approach, and the switch to row layout with adjusted alignments is well-implemented.
35-41: LGTM!The
.app-textcontainer properly supports the new HTML structure with flex-column layout and overflow handling for text truncation.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html (4)
9-26: LGTM!The header structure cleanly groups the title, optional subtitle, and close button. The close button has proper accessibility with
aria-labelbinding to the translated close text.
58-58: LGTM!Keyboard navigation handler correctly added to both router-link and default anchor app types, enabling arrow key navigation within the launchpad grid.
30-36: LGTM!The category structure properly manages spacing with conditional margins -
mt-4for the first category andmt-8for subsequent categories, providing appropriate visual separation.
90-100: LGTM!The show more/less button styling is updated with the
btn-show-allclass for styling control and increased vertical margins for better spacing.
projects/element-ng/application-header/testing/si-launchpad-category.harness.ts
Outdated
Show resolved
Hide resolved
dba42a0 to
332e91d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/app/examples/si-application-header/si-launchpad.ts:
- Around line 163-166: The updateFavorite method directly mutates app.favorite
before calling this.baseApps.set(...), which can be brittle; instead create an
immutable update by mapping the current array (this.baseApps()) and returning a
new array where the matching App (by id or unique key) is replaced with a new
object with favorite toggled/updated, then call this.baseApps.set(newArray).
Update the method updateFavorite({ app, favorite }) to produce and set that new
array rather than mutating app in place.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (12)
playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (29)
api-goldens/element-ng/application-header/index.api.mdapi-goldens/element-ng/translate/index.api.mdplaywright/e2e/element-examples/si-launchpad.spec.tsplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.htmlprojects/element-ng/application-header/si-application-header.component.scssprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/translate/si-translatable-keys.interface.tsprojects/element-theme/src/styles/components/_application-header.scssprojects/element-theme/src/styles/variables/_zindex.scsssrc/app/examples/si-application-header/si-launchpad.htmlsrc/app/examples/si-application-header/si-launchpad.ts
💤 Files with no reviewable changes (2)
- api-goldens/element-ng/translate/index.api.md
- projects/element-ng/translate/si-translatable-keys.interface.ts
🧰 Additional context used
🧠 Learnings (34)
📚 Learning: 2025-12-11T10:09:01.564Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1151
File: projects/element-ng/auto-collapsable-list/si-auto-collapsable-list.directive.spec.ts:83-104
Timestamp: 2025-12-11T10:09:01.564Z
Learning: In the auto-collapsable-list directive tests (projects/element-ng/auto-collapsable-list/si-auto-collapsable-list.directive.spec.ts), jasmine.clock() successfully controls the directive's setTimeout-based layout logic in zoneless mode. Mock timers work fine here, unlike in select component overflow tests where real setTimeout waits are required.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T11:54:31.132Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1133
File: projects/element-ng/tour/si-tour.service.spec.ts:21-26
Timestamp: 2025-12-04T11:54:31.132Z
Learning: In the siemens/element repository, all components are standalone by default and do not require the explicit `standalone: true` flag. Components should be added to the `imports` array in TestBed configuration, not the `declarations` array.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.tssrc/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2025-12-08T11:24:45.272Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/options/si-select-lazy-options.directive.spec.ts:67-85
Timestamp: 2025-12-08T11:24:45.272Z
Learning: In select lazy options tests (projects/element-ng/select/options/si-select-lazy-options.directive.spec.ts), jasmine.clock() cannot control RxJS debounceTime in zoneless mode. Real setTimeout waits must be used instead for tests involving search debouncing.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-08T11:25:20.861Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/si-select.component.spec.ts:204-205
Timestamp: 2025-12-08T11:25:20.861Z
Learning: In zoneless Angular tests (using provideZonelessChangeDetection), calling fixture.changeDetectorRef.markForCheck() before fixture.detectChanges() is required even for components using the default change detection strategy (not OnPush). This differs from zone.js-based testing where markForCheck is primarily needed only for OnPush components.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T05:50:17.637Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1135
File: projects/element-ng/tree-view/si-tree-view.component.spec.ts:953-964
Timestamp: 2025-12-04T05:50:17.637Z
Learning: In the siemens/element repository, when making tree-view tests zoneless-ready, jasmine.clock() should be used to mimic setTimeouts even in zoneless mode, as the component implementation relies on setTimeout calls that need to be controlled in tests.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T07:16:53.762Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts:92-105
Timestamp: 2025-12-15T07:16:53.762Z
Learning: In gridstack-wrapper component tests (projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts), real setTimeout waits must be used instead of fixture.whenStable() to avoid injector destroyed errors during mount/unmount operations in zoneless mode. The GridStack timing-dependent operations require actual async delays.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T05:50:38.735Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1135
File: projects/element-ng/tree-view/si-tree-view.component.spec.ts:1244-1245
Timestamp: 2025-12-04T05:50:38.735Z
Learning: In zoneless Angular tests with OnPush components, calling `await runOnPushChangeDetection(fixture)` followed by `await fixture.whenStable()` is NOT redundant. The `runOnPushChangeDetection` helper forces change detection to run for OnPush components, while `whenStable()` waits for all async operations to complete. Both are necessary and serve different purposes.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-02T06:50:25.778Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1104
File: projects/element-ng/datepicker/components/si-day-selection.component.spec.ts:19-19
Timestamp: 2025-12-02T06:50:25.778Z
Learning: The `runOnPushChangeDetection` helper is located at `projects/element-ng/test-helpers/change-detection.helper.ts` and is shared across the element-ng project, not scoped under individual feature modules like datepicker.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.ts
📚 Learning: 2025-12-01T04:01:27.365Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1099
File: projects/element-ng/accordion/si-accordion.component.spec.ts:39-43
Timestamp: 2025-12-01T04:01:27.365Z
Learning: With Angular 20 and esbuild, synchronous `TestBed.configureTestingModule(...).compileComponents()` calls work correctly even when components use external templates (templateUrl), because esbuild pre-compiles templates at build time. The async/await pattern is not required in this scenario.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-08T11:25:51.584Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/si-select.component.spec.ts:511-516
Timestamp: 2025-12-08T11:25:51.584Z
Learning: In the select component overflow tests (projects/element-ng/select/si-select.component.spec.ts), jasmine.clock() cannot be used to control the timing for overflow item detection. Real setTimeout waits must be used instead, likely due to ResizeObserver or debounced layout calculations that jasmine.clock() cannot control in zoneless mode.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-01T14:12:11.111Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1104
File: projects/element-ng/datepicker/components/si-day-selection.component.spec.ts:198-202
Timestamp: 2025-12-01T14:12:11.111Z
Learning: In zoneless Angular tests (using provideZonelessChangeDetection), prefer `await fixture.whenStable()` over `fixture.detectChanges()` after dispatching events or triggering async operations, as `whenStable()` waits for all asynchronous operations to complete and better mimics the production environment.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-05T08:00:38.407Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1151
File: projects/element-ng/typeahead/si-typeahead.directive.spec.ts:114-121
Timestamp: 2025-12-05T08:00:38.407Z
Learning: In the typeahead test suite (projects/element-ng/typeahead/si-typeahead.directive.spec.ts), jasmine.clock() must be installed in beforeAll and uninstalled in afterAll (not beforeEach/afterEach) because using per-test clock installation causes schedulerFunction-related issues in these specific tests.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T10:05:59.100Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1193
File: projects/element-ng/search-bar/si-search-bar.component.spec.ts:21-28
Timestamp: 2025-12-15T10:05:59.100Z
Learning: In tests that rely on Jasmine's fake clock to control time-based RxJS operators (e.g., debounceTime), use jasmine.clock().install() together with jasmine.clock().mockDate() to enable precise timing control. Without mockDate, scheduler-based operators may not respond to the fake clock. Apply this guidance to spec files (e.g., any file named *.spec.ts or *.spec.js) that test time-sensitive behavior, ensuring you install and mockDate before advancing time in tests.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsplaywright/e2e/element-examples/si-launchpad.spec.ts
📚 Learning: 2025-12-15T07:16:32.082Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts:69-80
Timestamp: 2025-12-15T07:16:32.082Z
Learning: In gridstack-wrapper component tests (projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts), real setTimeout waits (e.g., `await new Promise(resolve => setTimeout(resolve, 0))`) must be used instead of `fixture.whenStable()` when testing grid item rendering and initialization. This is likely due to gridstack library's initialization timing or lifecycle behavior that fixture.whenStable() cannot properly wait for in zoneless mode.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/si-application-header.component.spec.tssrc/app/examples/si-application-header/si-launchpad.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsplaywright/e2e/element-examples/si-launchpad.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-30T13:52:33.581Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1221
File: projects/element-ng/side-panel/si-side-panel.component.ts:136-142
Timestamp: 2025-12-30T13:52:33.581Z
Learning: Maintain the existing negative naming convention (e.g., disableBackdrop) across the siemens/element repository for consistency. Do not refactor to positive names (e.g., showBackdrop, hasBackdrop) unless the project explicitly changes the convention. In reviews, flag changes that alter established naming patterns in related files under projects/element-ng.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-15T07:17:06.981Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/widget-instance-editor-dialog/si-widget-instance-editor-dialog.component.spec.ts:37-49
Timestamp: 2025-12-15T07:17:06.981Z
Learning: In widget-instance-editor-dialog component tests (projects/dashboards-ng/src/components/widget-instance-editor-dialog/si-widget-instance-editor-dialog.component.spec.ts), real setTimeout waits (e.g., `await new Promise(resolve => setTimeout(resolve, 0))`) must be used instead of `fixture.whenStable()` when testing component initialization. This is likely due to the component's lifecycle behavior or modal initialization timing that fixture.whenStable() cannot properly wait for in zoneless mode.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.ts
📚 Learning: 2025-12-22T13:04:35.578Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: src/app/examples/ag-grid/ag-grid-empty-state.ts:20-24
Timestamp: 2025-12-22T13:04:35.578Z
Learning: The SiEmptyStateComponent in siemens/element-ng accepts icon names in kebab-case format (e.g., 'element-technical-operator') that may not be directly exported as constants in the element-icons.ts file. Do not flag these as errors if they render correctly at runtime.
Applied to files:
api-goldens/element-ng/application-header/index.api.mdprojects/element-ng/application-header/si-application-header.component.tssrc/app/examples/si-application-header/si-launchpad.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-23T09:24:35.163Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/translate/index.api.md:12-84
Timestamp: 2025-12-23T09:24:35.163Z
Learning: In the siemens/element repository, do not review files under the api-goldens/ directory (e.g., api-goldens/**/index.api.md) since they are auto-generated by API Extractor. Exclude these from review checks and focus on source files that are not auto-generated.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-23T09:24:57.843Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/index.api.md:324-325
Timestamp: 2025-12-23T09:24:57.843Z
Learning: Do not review files in the api-goldens directory, as they are auto-generated API reports produced by API Extractor.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-23T09:24:40.395Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/module-federation/index.api.md:7-11
Timestamp: 2025-12-23T09:24:40.395Z
Learning: In the siemens/element repository, do not review any files under the api-goldens directory, as they are auto-generated API reports produced by API Extractor. These MD files should be excluded from review unless explicitly overridden.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-11-26T13:50:02.735Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1090
File: projects/element-ng/typeahead/si-typeahead.directive.ts:511-515
Timestamp: 2025-11-26T13:50:02.735Z
Learning: In the typeahead component (projects/element-ng/typeahead/si-typeahead.directive.ts), the `createOption()` method should always close the dropdown with `this.canBeOpen.set(false)` regardless of whether `typeaheadMultiSelect` is enabled. This is different from `selectMatch()` which only closes the dropdown when NOT in multi-select mode.
Applied to files:
projects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/si-application-header.component.html
📚 Learning: 2025-12-09T14:33:54.441Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: src/app/examples/si-chat-messages/si-ai-message.ts:24-43
Timestamp: 2025-12-09T14:33:54.441Z
Learning: In src/app/examples/ (example/demo code) directories, allow and expect code duplication. Treat duplication as acceptable for demonstration code that aims to illustrate usage rather than enforce DRY. Do not enforce refactoring to remove duplication in example files; focus on readability and clarity of examples. This applies to all TypeScript files under src/app/examples (including nested subdirectories like si-chat-messages).
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2026-01-07T08:57:45.608Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1226
File: src/app/examples/si-navbar-vertical/si-navbar-vertical.ts:112-115
Timestamp: 2026-01-07T08:57:45.608Z
Learning: Do not strictly enforce badge color consistency in example files under src/app/examples. Treat badge color checks as non-blocking for TypeScript files in this directory; reviewers may still note inconsistencies, but they should not require changes for these example files. Apply this guidance broadly to all files within src/app/examples.
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2025-12-16T16:19:17.950Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1207
File: playwright/snapshots/si-filtered-search.spec.ts-snapshots/si-filtered-search--si-filtered-search-playground--data-entered.yaml:38-38
Timestamp: 2025-12-16T16:19:17.950Z
Learning: Do not review files in the playwright/snapshots directory, as they contain auto-generated test artifacts. Exclude any YAML snapshot files under playwright/snapshots from code reviews in the siemens/element repository. If a YAML file is not an auto-generated snapshot, verify its origin before skipping.
Applied to files:
playwright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yaml
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2026-01-02T06:41:22.404Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: playwright/e2e/element-examples/static.spec.ts:119-121
Timestamp: 2026-01-02T06:41:22.404Z
Learning: In the Element design system (siemens/element repository), disabling the `aria-required-children` accessibility rule for `SiEmptyStateComponent` is an accepted pattern. This component is used in AG Grid overlays (AgNoRowsOverlayComponent) and other places in Element, and the rule is consistently disabled in tests where this component appears (e.g., si-pills-input, datatable-row-group, ag-grid-empty-state).
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.ts
📚 Learning: 2026-01-02T16:41:57.274Z
Learnt from: kfenner
Repo: siemens/element PR: 1005
File: docs/fundamentals/styles/sizing.md:18-26
Timestamp: 2026-01-02T16:41:57.274Z
Learning: In Element documentation (siemens/element), sizing utility examples in docs may intentionally differ from implementation examples to demonstrate multiple valid approaches - for example, documentation might use traditional CSS properties like `height` and `width` while the implementation example uses logical properties like `block-size` and `inline-size`. These are considered different examples rather than inconsistencies.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-22T13:29:36.922Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/ag-grid/parts/color-scheme.part.ts:35-35
Timestamp: 2025-12-22T13:29:36.922Z
Learning: In the Element design system (projects/element-ng/theme and projects/element-theme), only `--element-body-font-family` is exposed as a CSS custom property for typography. Font sizes are NOT exposed as CSS custom properties; they are defined using internal SCSS variables (e.g., `typography.$si-font-size-body`) that compile to fixed values. Using hardcoded pixel values for fontSize in AG Grid theme configuration (e.g., `fontSize: '14px'`) is the correct approach.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-31T07:16:13.451Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/ag-grid/parts/radio-style.part.ts:15-61
Timestamp: 2025-12-31T07:16:13.451Z
Learning: In the AG Grid theme package (projects/element-ng/ag-grid), styling should maintain consistency with the base Element design system components rather than adding enhancements or extra CSS. If accessibility improvements like focus-visible indicators are needed, they should be implemented at the Element design system level first.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-17T14:01:05.705Z
Learnt from: dauriamarco
Repo: siemens/element PR: 908
File: projects/element-ng/side-panel/si-side-panel.component.ts:70-74
Timestamp: 2025-12-17T14:01:05.705Z
Learning: In the Element side-panel component (projects/element-ng/side-panel/si-side-panel.component.ts), the `xxxlMinimum` breakpoint (1920px) is intentionally hardcoded as a private static constant rather than being part of the configurable `Breakpoints` interface. This design preserves full Bootstrap compatibility (which only defines breakpoints up to xxl/1400px) while allowing Element to handle ultra-wide screens explicitly without affecting the standard breakpoint configuration.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-03T13:02:34.875Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1058
File: docs/fundamentals/layouts/breakpoints.md:25-31
Timestamp: 2025-12-03T13:02:34.875Z
Learning: In Element documentation for CSS breakpoints and class infixes (e.g., in docs/fundamentals/layouts/breakpoints.md), _None_ is formatted with italics to indicate the absence of a class infix (representing base mobile-first styles), while actual CSS class infixes like `sm`, `md`, `lg`, `xl`, `xxl` use inline code formatting with backticks. This semantic formatting distinction is intentional.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-30T13:52:41.363Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1221
File: projects/element-ng/side-panel/si-side-panel.component.ts:136-142
Timestamp: 2025-12-30T13:52:41.363Z
Learning: In the siemens/element repository, the library uses negative naming conventions like `disableBackdrop` intentionally for consistency across the codebase. This pattern should be maintained rather than converting to positive naming conventions like `showBackdrop` or `hasBackdrop`.
Applied to files:
projects/element-ng/application-header/si-application-header.component.html
📚 Learning: 2025-12-09T14:32:34.036Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:21-47
Timestamp: 2025-12-09T14:32:34.036Z
Learning: In projects/element-ng/chat-messages, nested if blocks are preferred over switch statements for branching on message types in Angular templates.
Applied to files:
projects/element-ng/application-header/si-application-header.component.html
🧬 Code graph analysis (2)
src/app/examples/si-application-header/si-launchpad.ts (1)
projects/element-ng/application-header/launchpad/si-launchpad.model.ts (2)
App(53-53)AppCategory(31-36)
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (1)
projects/element-ng/common/helpers/rtl.ts (1)
correctKeyRTL(9-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (36)
projects/element-theme/src/styles/components/_application-header.scss (1)
34-38: LGTM! Previous feedback properly addressed.The comment has been updated to accurately reflect the current implementation. The border-hiding logic now correctly uses two independent conditions (
&:has(.dropdown-menu.show)and&.show-navigation), and the comment clearly describes this behavior. This aligns well with the component's signal-based state management.projects/element-theme/src/styles/variables/_zindex.scss (1)
19-19: No changes needed. The z-index reordering is correct and intentional.Dropdowns (1065), tooltips (1080), and popovers (1070) all use global z-index values that are higher than the application header (1033) and launchpad (1032), so they will render above both regardless of this change. The launchpad component does not contain custom elevated overlays that would be affected by this reordering.
Likely an incorrect or invalid review comment.
projects/element-ng/application-header/si-application-header.component.scss (2)
3-6: LGTM! Backdrop opacity prepared for animation control.The
opacity: 0setting allows the Angular animation ([@backdrop] trigger mentioned in the component) to control the backdrop's visibility transition. This is a standard pattern for animated overlays.
8-13: LGTM! Proper overlay container setup.The new
.launchpad-containerfollows standard modal overlay patterns:
- Full viewport coverage with
inset: 0pointer-events: noneallows interaction to pass through when inactive (child elements can override)- Z-index from theme variables ensures correct layering
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.ts (1)
15-15: LGTM! Standard test setup with noop animations.Adding
provideNoopAnimationsis a best practice for unit tests:
- Prevents animation timing issues
- Improves test reliability and speed
- Aligns with the broader pattern of animation handling in this PR
Also applies to: 51-56
projects/element-ng/application-header/testing/si-launchpad.harness.ts (1)
19-21: LGTM! Category name updated to match simplified labeling.The change from
'Favorite apps'to'Favorites'aligns with the broader simplification of launchpad text and labels across the PR. The shorter, cleaner label is more concise while maintaining clarity.projects/element-ng/application-header/si-application-header.component.spec.ts (1)
15-15: LGTM! Consistent animation provider setup across test suites.Adding
provideNoopAnimations()to all test blocks is the correct approach for testing components that now include Angular animations (@expandand@backdroptriggers). This ensures tests run without waiting for real animation timings.Also applies to: 106-106, 222-222, 310-311, 361-361
projects/element-ng/application-header/si-application-header.component.ts (2)
5-11: Good practice: Aliasingtriggerto avoid naming conflicts.Aliasing
triggerasanimationTriggerprevents potential conflicts with RxJS or other imports that might use the same name. This is a clean approach.
51-80: Animation definitions look correct.The animation triggers are well-structured:
- Asymmetric timing (0.5s enter, 0.25s leave) provides a snappy feel when closing
- The backdrop fade uses a consistent linear transition
Consider documenting the
-120pxoffset if it correlates with a design token or header height, but this is minor.playwright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yaml (1)
1-51: Skipping auto-generated snapshot file.Based on repository conventions, files in
playwright/snapshotsare auto-generated test artifacts and should not be reviewed.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html (4)
10-25: Well-structured header with accessibility support.The new
apps-headerblock properly groups the title/subtitle with the close button. The close button correctly usesaria-labelfor screen reader accessibility.
58-58: Keyboard navigation handlers added consistently.The
handleAppsNavigationhandler is correctly attached to both router-link and default anchor app items, enabling keyboard navigation across the launchpad grid.Also applies to: 77-77
27-36: Spacing adjustments align with PR feedback.The updated margin classes and the
ms-n2 ps-2pattern for the scroll container provide proper visual alignment. The dynamic top margin (mt-4for first,mt-8for subsequent) creates appropriate visual hierarchy between categories.
91-99: Toggle button styling updated.The
btn-show-allclass andmy-6spacing updates match the broader styling changes in this PR.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss (3)
3-3: Import updated to all-variables.The switch to
all-variablesprovides access to the full set of design tokens, enabling consistent use of spacers, breakpoints, and colors throughout the file.
5-27: App-switcher positioning and spacing updated.The change from
fixedtoabsolutepositioning, combined with the inset calculations and responsive padding adjustments, aligns the launchpad container with the new responsive design requirements.
29-33: New.apps-headerlayout supports header structure.The flex layout with
space-betweencorrectly positions the title and close button at opposite ends of the header.projects/element-ng/application-header/launchpad/si-launchpad-app.component.ts (2)
48-48: Formatting-only change.This is a whitespace adjustment that improves code organization by visually separating the action property from the iconUrl/iconClass properties.
37-66: The disabled state has been successfully removed from the launchpad feature. The AppBase interface in si-launchpad.model.ts contains no disabled property, and SiLaunchpadAppComponent has no disabled input defined. No disabled references exist in the launchpad components.projects/element-ng/application-header/launchpad/si-launchpad.spec.ts (1)
115-115: LGTM! Test expectation updated to match new UI text.The updated expectation aligns with the new "Favorites" label used throughout the launchpad UI.
playwright/e2e/element-examples/si-launchpad.spec.ts (1)
16-16: LGTM! Test updated to mark Fischbach as favorite.The change from 'Rocket' to 'Fischbach' appears intentional and aligns with the updated test snapshots.
projects/element-ng/application-header/si-application-header.component.html (2)
56-60: LGTM! Launchpad container with animation trigger added correctly.The new wrapper structure with the
[@expand]animation trigger supports the responsive behavior introduced in this PR.
67-67: LGTM! Backdrop animation trigger added.The animation binding correctly supports the show state for the backdrop element.
projects/element-ng/application-header/launchpad/si-launchpad-app.component.html (1)
6-18: Spacing between App name and System name is correct.The
mt-2class on line 15 provides the required 4px spacing. This matches the design system specification wherespacing-2is defined as 4px.projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.ts (4)
1-32: Well-structured test setup for zoneless mode.The test configuration correctly uses
provideZonelessChangeDetection()and properly mocks theSiApplicationHeaderComponentdependency. The TestHostComponent provides a clean mounting point for testing the navigation logic.
34-45: Effective mock strategy for layout testing.The
createMockAppshelper appropriately mocksgetBoundingClientRectto simulate different layout configurations, enabling deterministic testing of position-based navigation logic.
47-109: Comprehensive navigation test coverage.The test suite thoroughly exercises the vertical navigation logic across multiple scenarios:
- Single column layouts with wrapping behavior
- Grid layouts (2x3) with row-based navigation
- Edge cases including wrap-around at boundaries
- Single-row layouts where vertical navigation should not occur
The data-driven approach using test case arrays makes the tests clear and maintainable.
111-133: Good coverage of tolerance handling.The tests verify that the navigation algorithm correctly applies tolerance thresholds:
leftTolerance(20px) for detecting single-column layouts despite minor horizontal variationsrowTolerance(10px) for grouping items into rows with slight vertical misalignmentThis ensures the navigation works robustly with real-world layout imperfections.
src/app/examples/si-application-header/si-launchpad.html (2)
1-79: Well-structured configuration UI with good accessibility.The Launchpad Configuration section provides a clear, accessible interface for testing launchpad features:
- Proper semantic HTML with descriptive labels
- Accessibility attributes (
role="switch", label associations)- Responsive layout using Bootstrap grid
- Clear visual hierarchy and helpful demo note
82-86: Correct signal binding syntax.The launchpad factory bindings correctly use function call syntax (
apps(),enableFavorites()) to access signal values in the template, aligning with Angular's signals API.projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss (2)
20-32: Well-structured hover and active state separation.The separation of
.activeand:hoverstates withbase-1-selectedfor active andbase-1-hoverfor hover addresses the PR feedback correctly. This provides clear visual distinction between selected and hovered items.
77-102: Clean responsive implementation for mobile layout.The breakpoint correctly switches from column to row layout, adjusts spacing, and repositions the favorite icon. The use of
:has(.favorite-icon)for conditional padding is a nice touch.src/app/examples/si-application-header/si-launchpad.ts (2)
29-31: Good use of Angular Signals for reactive state management.The signals-based approach with
enableFavoritesandenableCategoriesprovides clean reactive state management that integrates well with Angular's OnPush change detection.
121-161: Computed categorization logic is clear for demonstration purposes.The computed
appsgetter effectively demonstrates both flat and categorized views. While the hardcoded name-based filtering (lines 127-135) isn't ideal for production code, it's acceptable for example code that aims to illustrate the categorization feature.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (2)
61-64: Verify the title wording with stakeholders.Per PR discussion, there's an ongoing debate about whether to use "Switch to application" vs. "Switch to app". @Danisand provided context distinguishing "Apps" (browser/mobile products) from "Applications" (installed on controllers). Ensure this wording is finalized before merging.
163-201: Well-implemented keyboard navigation with RTL support.The navigation logic correctly:
- Uses
correctKeyRTLfor RTL language support- Handles horizontal navigation with wrap-around (line 189)
- Delegates vertical navigation to the spatial algorithm
- Uses
:scope >for proper selector isolation (addressing past review feedback)
332e91d to
b8f0a3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss:
- Around line 35-41: The .app-text flex container lacks vertical spacing between
its stacked children; update the .app-text rule (in
si-launchpad-app.component.scss) to add a 4px gap (e.g., gap: 4px or gap:
var(--spacing-2)) so there is the required spacing between the App name and
System name while keeping the existing display, align-items and flex-direction
settings.
In
@projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts:
- Around line 143-145: Add brief inline documentation above the two numeric
tolerance constants so their purpose and units are clear: document that
rowTolerance = 10 (pixels) is the vertical tolerance used to treat items as
being in the same row for keyboard arrow navigation/layout detection, and
leftTolerance = 20 (pixels) is the horizontal tolerance used when deciding
left/right focus movement; reference the constants rowTolerance and
leftTolerance in the comment so future readers know these magic numbers relate
to keyboard navigation and layout heuristics.
In
@projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.ts:
- Around line 34-45: The test helper createMockApps currently stubs
getBoundingClientRect with a partial DOMRect; update the mock in createMockApps
(the spyOn(app, 'getBoundingClientRect') returnValue) to return a full
DOMRect-shaped object including properties top, left, width, height, right,
bottom, x, y and optionally toJSON so the returned object strictly matches the
DOMRect type and improves type safety for future test changes.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (12)
playwright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (29)
api-goldens/element-ng/application-header/index.api.mdapi-goldens/element-ng/translate/index.api.mdplaywright/e2e/element-examples/si-launchpad.spec.tsplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.htmlprojects/element-ng/application-header/si-application-header.component.scssprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/translate/si-translatable-keys.interface.tsprojects/element-theme/src/styles/components/_application-header.scssprojects/element-theme/src/styles/variables/_zindex.scsssrc/app/examples/si-application-header/si-launchpad.htmlsrc/app/examples/si-application-header/si-launchpad.ts
💤 Files with no reviewable changes (2)
- projects/element-ng/translate/si-translatable-keys.interface.ts
- api-goldens/element-ng/translate/index.api.md
🧰 Additional context used
🧠 Learnings (34)
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.
Applied to files:
projects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsplaywright/e2e/element-examples/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tssrc/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2025-12-30T13:52:33.581Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1221
File: projects/element-ng/side-panel/si-side-panel.component.ts:136-142
Timestamp: 2025-12-30T13:52:33.581Z
Learning: Maintain the existing negative naming convention (e.g., disableBackdrop) across the siemens/element repository for consistency. Do not refactor to positive names (e.g., showBackdrop, hasBackdrop) unless the project explicitly changes the convention. In reviews, flag changes that alter established naming patterns in related files under projects/element-ng.
Applied to files:
projects/element-ng/application-header/testing/si-launchpad-category.harness.tsprojects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/testing/si-launchpad.harness.tsprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-22T13:04:35.578Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: src/app/examples/ag-grid/ag-grid-empty-state.ts:20-24
Timestamp: 2025-12-22T13:04:35.578Z
Learning: The SiEmptyStateComponent in siemens/element-ng accepts icon names in kebab-case format (e.g., 'element-technical-operator') that may not be directly exported as constants in the element-icons.ts file. Do not flag these as errors if they render correctly at runtime.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsapi-goldens/element-ng/application-header/index.api.mdprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/si-application-header.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.tssrc/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2026-01-02T06:41:22.404Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: playwright/e2e/element-examples/static.spec.ts:119-121
Timestamp: 2026-01-02T06:41:22.404Z
Learning: In the Element design system (siemens/element repository), disabling the `aria-required-children` accessibility rule for `SiEmptyStateComponent` is an accepted pattern. This component is used in AG Grid overlays (AgNoRowsOverlayComponent) and other places in Element, and the rule is consistently disabled in tests where this component appears (e.g., si-pills-input, datatable-row-group, ag-grid-empty-state).
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.ts
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scssprojects/element-ng/application-header/launchpad/si-launchpad-app.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-app.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2026-01-02T16:41:57.274Z
Learnt from: kfenner
Repo: siemens/element PR: 1005
File: docs/fundamentals/styles/sizing.md:18-26
Timestamp: 2026-01-02T16:41:57.274Z
Learning: In Element documentation (siemens/element), sizing utility examples in docs may intentionally differ from implementation examples to demonstrate multiple valid approaches - for example, documentation might use traditional CSS properties like `height` and `width` while the implementation example uses logical properties like `block-size` and `inline-size`. These are considered different examples rather than inconsistencies.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-22T13:29:36.922Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/ag-grid/parts/color-scheme.part.ts:35-35
Timestamp: 2025-12-22T13:29:36.922Z
Learning: In the Element design system (projects/element-ng/theme and projects/element-theme), only `--element-body-font-family` is exposed as a CSS custom property for typography. Font sizes are NOT exposed as CSS custom properties; they are defined using internal SCSS variables (e.g., `typography.$si-font-size-body`) that compile to fixed values. Using hardcoded pixel values for fontSize in AG Grid theme configuration (e.g., `fontSize: '14px'`) is the correct approach.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-31T07:16:13.451Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/ag-grid/parts/radio-style.part.ts:15-61
Timestamp: 2025-12-31T07:16:13.451Z
Learning: In the AG Grid theme package (projects/element-ng/ag-grid), styling should maintain consistency with the base Element design system components rather than adding enhancements or extra CSS. If accessibility improvements like focus-visible indicators are needed, they should be implemented at the Element design system level first.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-17T14:01:05.705Z
Learnt from: dauriamarco
Repo: siemens/element PR: 908
File: projects/element-ng/side-panel/si-side-panel.component.ts:70-74
Timestamp: 2025-12-17T14:01:05.705Z
Learning: In the Element side-panel component (projects/element-ng/side-panel/si-side-panel.component.ts), the `xxxlMinimum` breakpoint (1920px) is intentionally hardcoded as a private static constant rather than being part of the configurable `Breakpoints` interface. This design preserves full Bootstrap compatibility (which only defines breakpoints up to xxl/1400px) while allowing Element to handle ultra-wide screens explicitly without affecting the standard breakpoint configuration.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-03T13:02:34.875Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1058
File: docs/fundamentals/layouts/breakpoints.md:25-31
Timestamp: 2025-12-03T13:02:34.875Z
Learning: In Element documentation for CSS breakpoints and class infixes (e.g., in docs/fundamentals/layouts/breakpoints.md), _None_ is formatted with italics to indicate the absence of a class infix (representing base mobile-first styles), while actual CSS class infixes like `sm`, `md`, `lg`, `xl`, `xxl` use inline code formatting with backticks. This semantic formatting distinction is intentional.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
📚 Learning: 2025-12-11T10:09:01.564Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1151
File: projects/element-ng/auto-collapsable-list/si-auto-collapsable-list.directive.spec.ts:83-104
Timestamp: 2025-12-11T10:09:01.564Z
Learning: In the auto-collapsable-list directive tests (projects/element-ng/auto-collapsable-list/si-auto-collapsable-list.directive.spec.ts), jasmine.clock() successfully controls the directive's setTimeout-based layout logic in zoneless mode. Mock timers work fine here, unlike in select component overflow tests where real setTimeout waits are required.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-08T11:24:45.272Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/options/si-select-lazy-options.directive.spec.ts:67-85
Timestamp: 2025-12-08T11:24:45.272Z
Learning: In select lazy options tests (projects/element-ng/select/options/si-select-lazy-options.directive.spec.ts), jasmine.clock() cannot control RxJS debounceTime in zoneless mode. Real setTimeout waits must be used instead for tests involving search debouncing.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T07:16:53.762Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts:92-105
Timestamp: 2025-12-15T07:16:53.762Z
Learning: In gridstack-wrapper component tests (projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts), real setTimeout waits must be used instead of fixture.whenStable() to avoid injector destroyed errors during mount/unmount operations in zoneless mode. The GridStack timing-dependent operations require actual async delays.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-08T11:25:51.584Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/si-select.component.spec.ts:511-516
Timestamp: 2025-12-08T11:25:51.584Z
Learning: In the select component overflow tests (projects/element-ng/select/si-select.component.spec.ts), jasmine.clock() cannot be used to control the timing for overflow item detection. Real setTimeout waits must be used instead, likely due to ResizeObserver or debounced layout calculations that jasmine.clock() cannot control in zoneless mode.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
📚 Learning: 2025-12-08T11:25:20.861Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1163
File: projects/element-ng/select/si-select.component.spec.ts:204-205
Timestamp: 2025-12-08T11:25:20.861Z
Learning: In zoneless Angular tests (using provideZonelessChangeDetection), calling fixture.changeDetectorRef.markForCheck() before fixture.detectChanges() is required even for components using the default change detection strategy (not OnPush). This differs from zone.js-based testing where markForCheck is primarily needed only for OnPush components.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T05:50:17.637Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1135
File: projects/element-ng/tree-view/si-tree-view.component.spec.ts:953-964
Timestamp: 2025-12-04T05:50:17.637Z
Learning: In the siemens/element repository, when making tree-view tests zoneless-ready, jasmine.clock() should be used to mimic setTimeouts even in zoneless mode, as the component implementation relies on setTimeout calls that need to be controlled in tests.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-04T05:50:38.735Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1135
File: projects/element-ng/tree-view/si-tree-view.component.spec.ts:1244-1245
Timestamp: 2025-12-04T05:50:38.735Z
Learning: In zoneless Angular tests with OnPush components, calling `await runOnPushChangeDetection(fixture)` followed by `await fixture.whenStable()` is NOT redundant. The `runOnPushChangeDetection` helper forces change detection to run for OnPush components, while `whenStable()` waits for all async operations to complete. Both are necessary and serve different purposes.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-02T06:50:25.778Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1104
File: projects/element-ng/datepicker/components/si-day-selection.component.spec.ts:19-19
Timestamp: 2025-12-02T06:50:25.778Z
Learning: The `runOnPushChangeDetection` helper is located at `projects/element-ng/test-helpers/change-detection.helper.ts` and is shared across the element-ng project, not scoped under individual feature modules like datepicker.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.ts
📚 Learning: 2025-12-01T04:01:27.365Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1099
File: projects/element-ng/accordion/si-accordion.component.spec.ts:39-43
Timestamp: 2025-12-01T04:01:27.365Z
Learning: With Angular 20 and esbuild, synchronous `TestBed.configureTestingModule(...).compileComponents()` calls work correctly even when components use external templates (templateUrl), because esbuild pre-compiles templates at build time. The async/await pattern is not required in this scenario.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-01T14:12:11.111Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1104
File: projects/element-ng/datepicker/components/si-day-selection.component.spec.ts:198-202
Timestamp: 2025-12-01T14:12:11.111Z
Learning: In zoneless Angular tests (using provideZonelessChangeDetection), prefer `await fixture.whenStable()` over `fixture.detectChanges()` after dispatching events or triggering async operations, as `whenStable()` waits for all asynchronous operations to complete and better mimics the production environment.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-05T08:00:38.407Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1151
File: projects/element-ng/typeahead/si-typeahead.directive.spec.ts:114-121
Timestamp: 2025-12-05T08:00:38.407Z
Learning: In the typeahead test suite (projects/element-ng/typeahead/si-typeahead.directive.spec.ts), jasmine.clock() must be installed in beforeAll and uninstalled in afterAll (not beforeEach/afterEach) because using per-test clock installation causes schedulerFunction-related issues in these specific tests.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T10:05:59.100Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1193
File: projects/element-ng/search-bar/si-search-bar.component.spec.ts:21-28
Timestamp: 2025-12-15T10:05:59.100Z
Learning: In tests that rely on Jasmine's fake clock to control time-based RxJS operators (e.g., debounceTime), use jasmine.clock().install() together with jasmine.clock().mockDate() to enable precise timing control. Without mockDate, scheduler-based operators may not respond to the fake clock. Apply this guidance to spec files (e.g., any file named *.spec.ts or *.spec.js) that test time-sensitive behavior, ensuring you install and mockDate before advancing time in tests.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad.spec.tsplaywright/e2e/element-examples/si-launchpad.spec.ts
📚 Learning: 2025-12-15T07:16:32.082Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts:69-80
Timestamp: 2025-12-15T07:16:32.082Z
Learning: In gridstack-wrapper component tests (projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.spec.ts), real setTimeout waits (e.g., `await new Promise(resolve => setTimeout(resolve, 0))`) must be used instead of `fixture.whenStable()` when testing grid item rendering and initialization. This is likely due to gridstack library's initialization timing or lifecycle behavior that fixture.whenStable() cannot properly wait for in zoneless mode.
Applied to files:
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.tsprojects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.tsprojects/element-ng/application-header/si-application-header.component.spec.ts
📚 Learning: 2025-12-15T07:17:06.981Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1191
File: projects/dashboards-ng/src/components/widget-instance-editor-dialog/si-widget-instance-editor-dialog.component.spec.ts:37-49
Timestamp: 2025-12-15T07:17:06.981Z
Learning: In widget-instance-editor-dialog component tests (projects/dashboards-ng/src/components/widget-instance-editor-dialog/si-widget-instance-editor-dialog.component.spec.ts), real setTimeout waits (e.g., `await new Promise(resolve => setTimeout(resolve, 0))`) must be used instead of `fixture.whenStable()` when testing component initialization. This is likely due to the component's lifecycle behavior or modal initialization timing that fixture.whenStable() cannot properly wait for in zoneless mode.
Applied to files:
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.ts
📚 Learning: 2025-12-23T09:24:35.163Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/translate/index.api.md:12-84
Timestamp: 2025-12-23T09:24:35.163Z
Learning: In the siemens/element repository, do not review files under the api-goldens/ directory (e.g., api-goldens/**/index.api.md) since they are auto-generated by API Extractor. Exclude these from review checks and focus on source files that are not auto-generated.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-23T09:24:57.843Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/index.api.md:324-325
Timestamp: 2025-12-23T09:24:57.843Z
Learning: Do not review files in the api-goldens directory, as they are auto-generated API reports produced by API Extractor.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-23T09:24:40.395Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1235
File: api-goldens/dashboards-ng/module-federation/index.api.md:7-11
Timestamp: 2025-12-23T09:24:40.395Z
Learning: In the siemens/element repository, do not review any files under the api-goldens directory, as they are auto-generated API reports produced by API Extractor. These MD files should be excluded from review unless explicitly overridden.
Applied to files:
api-goldens/element-ng/application-header/index.api.md
📚 Learning: 2025-12-16T16:19:17.950Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1207
File: playwright/snapshots/si-filtered-search.spec.ts-snapshots/si-filtered-search--si-filtered-search-playground--data-entered.yaml:38-38
Timestamp: 2025-12-16T16:19:17.950Z
Learning: Do not review files in the playwright/snapshots directory, as they contain auto-generated test artifacts. Exclude any YAML snapshot files under playwright/snapshots from code reviews in the siemens/element repository. If a YAML file is not an auto-generated snapshot, verify its origin before skipping.
Applied to files:
playwright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--mobile.yamlplaywright/snapshots/si-launchpad.spec.ts-snapshots/si-application-header--si-launchpad--new-favorite.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad--medium-size.yamlplaywright/snapshots/navbar-launchpad.spec.ts-snapshots/si-navbar--si-navbar-launchpad-categories.yaml
📚 Learning: 2025-12-04T11:54:31.132Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1133
File: projects/element-ng/tour/si-tour.service.spec.ts:21-26
Timestamp: 2025-12-04T11:54:31.132Z
Learning: In the siemens/element repository, all components are standalone by default and do not require the explicit `standalone: true` flag. Components should be added to the `imports` array in TestBed configuration, not the `declarations` array.
Applied to files:
projects/element-ng/application-header/si-application-header.component.spec.tssrc/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2025-11-26T13:50:02.735Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1090
File: projects/element-ng/typeahead/si-typeahead.directive.ts:511-515
Timestamp: 2025-11-26T13:50:02.735Z
Learning: In the typeahead component (projects/element-ng/typeahead/si-typeahead.directive.ts), the `createOption()` method should always close the dropdown with `this.canBeOpen.set(false)` regardless of whether `typeaheadMultiSelect` is enabled. This is different from `selectMatch()` which only closes the dropdown when NOT in multi-select mode.
Applied to files:
projects/element-ng/application-header/si-application-header.component.htmlprojects/element-ng/application-header/si-application-header.component.ts
📚 Learning: 2025-12-30T13:52:41.363Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1221
File: projects/element-ng/side-panel/si-side-panel.component.ts:136-142
Timestamp: 2025-12-30T13:52:41.363Z
Learning: In the siemens/element repository, the library uses negative naming conventions like `disableBackdrop` intentionally for consistency across the codebase. This pattern should be maintained rather than converting to positive naming conventions like `showBackdrop` or `hasBackdrop`.
Applied to files:
projects/element-ng/application-header/si-application-header.component.html
📚 Learning: 2025-12-09T14:32:34.036Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:21-47
Timestamp: 2025-12-09T14:32:34.036Z
Learning: In projects/element-ng/chat-messages, nested if blocks are preferred over switch statements for branching on message types in Angular templates.
Applied to files:
projects/element-ng/application-header/si-application-header.component.html
📚 Learning: 2025-12-09T14:33:54.441Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: src/app/examples/si-chat-messages/si-ai-message.ts:24-43
Timestamp: 2025-12-09T14:33:54.441Z
Learning: In src/app/examples/ (example/demo code) directories, allow and expect code duplication. Treat duplication as acceptable for demonstration code that aims to illustrate usage rather than enforce DRY. Do not enforce refactoring to remove duplication in example files; focus on readability and clarity of examples. This applies to all TypeScript files under src/app/examples (including nested subdirectories like si-chat-messages).
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
📚 Learning: 2026-01-07T08:57:45.608Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1226
File: src/app/examples/si-navbar-vertical/si-navbar-vertical.ts:112-115
Timestamp: 2026-01-07T08:57:45.608Z
Learning: Do not strictly enforce badge color consistency in example files under src/app/examples. Treat badge color checks as non-blocking for TypeScript files in this directory; reviewers may still note inconsistencies, but they should not require changes for these example files. Apply this guidance broadly to all files within src/app/examples.
Applied to files:
src/app/examples/si-application-header/si-launchpad.ts
🧬 Code graph analysis (3)
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.ts (1)
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (1)
Component(30-277)
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (2)
projects/element-translate-ng/translate/t.ts (1)
t(38-51)projects/element-ng/common/helpers/rtl.ts (1)
correctKeyRTL(9-14)
src/app/examples/si-application-header/si-launchpad.ts (1)
projects/element-ng/application-header/launchpad/si-launchpad.model.ts (2)
App(53-53)AppCategory(31-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (34)
projects/element-ng/application-header/testing/si-launchpad-category.harness.ts (1)
18-18: Excellent improvement to test stability!The selector has been correctly updated to use a
data-testidattribute instead of the previous fragile style class selector (.si-h4.mb-6). This change directly addresses the previous review feedback and follows testing best practices by:
- Using a semantic, purpose-built test attribute
- Eliminating brittleness from styling changes (especially utility classes like
.mb-6)- Making the harness more maintainable
projects/element-theme/src/styles/components/_application-header.scss (2)
34-38: Past review comment properly addressed.The comment now accurately reflects the implementation. It correctly describes that the border is hidden when either the dropdown menu is shown (
:has(.dropdown-menu.show)) or navigation is expanded (show-navigationclass).
35-35: No action needed. The:has()pseudo-class is already consistently used throughout the element-theme styles (5 occurrences across multiple component files), indicating this is an established pattern in the project's CSS architecture. The modern tooling stack and absence of a browserslist configuration confirm the project does not target legacy browser support.projects/element-theme/src/styles/variables/_zindex.scss (1)
19-19: LGTM. The z-index adjustment correctly positions the launchpad container (1032) below the application header (1033) and above the backdrop (1031). The stacking order is properly maintained through the variable relationships and is used consistently in the component styles.projects/element-ng/application-header/si-application-header.component.spec.ts (1)
15-15: LGTM: No-op animations provider added to test suitesThe addition of
provideNoopAnimations()across all test configurations is a best practice for:
- Eliminating animation timing issues in tests
- Speeding up test execution
- Making tests more deterministic
This aligns well with the PR's introduction of expand/backdrop animations for the launchpad feature.
Also applies to: 106-106, 222-222, 310-311, 361-361
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.ts (1)
15-15: LGTM: Consistent no-op animations configurationThe addition of
provideNoopAnimations()to the test providers is correct and consistent with the other test file changes in this PR. This will ensure animations are disabled during tests, improving test performance and reliability.Also applies to: 51-56
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.ts (2)
21-32: LGTM! Test setup is appropriate.The test configuration correctly uses
provideZonelessChangeDetection()and provides a minimal mock forSiApplicationHeaderComponent. Since these tests focus solely on navigation logic and don't exercise methods likecloseLaunchpad(), the empty object mock is sufficient.
47-134: Excellent test coverage for keyboard navigation logic.The tests comprehensively cover the
getVerticalTargetIndexmethod with scenarios for:
- Single-column layouts with wrap-around behavior
- Grid layouts (2x3) with various navigation directions
- Tolerance-based detection for column and row alignment
- Edge case of single-row navigation (no vertical movement)
Note: The tests access the private method via
(component as any).getVerticalTargetIndex(...). While this approach is acceptable for thorough unit testing of complex logic, it may require updates if the method signature or accessibility changes during refactoring.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss (3)
8-26: LGTM! Variable updates and responsive padding are well-implemented.The changes include:
- ✅ Border compensation comment added (addresses past review feedback)
- ✅ Consistent migration to
all-variablesimports- ✅ Responsive padding for medium breakpoint improves mobile/tablet UX
The styling adjustments align well with the new launchpad structure and responsive design goals mentioned in the PR objectives.
29-33: LGTM! New header structure supports the redesigned layout.The
.apps-headerflexbox layout effectively positions the launchpad title and close button with proper spacing and alignment, aligning with the PR's style and responsive design goals.
6-7: Verify the position change from fixed to absolute.The
position: absolutechange works correctly in conjunction with the.launchpad-container(fixed positioning) insi-application-header.component.scss. The structure creates proper layering: the fixed container provides the fullscreen viewport overlay withpointer-events: none, while the absolute.app-switcheris positioned within it withpointer-events: all, enabling proper event delegation. Theinset-block-startcalculation using header height and spacing variables ensures correct visual placement across viewport sizes and scroll positions.projects/element-ng/application-header/si-application-header.component.scss (2)
8-13: LGTM! Clean container structure for the launchpad overlay.The
.launchpad-containerprovides a well-designed fixed-position overlay:
position: fixedwithinset: 0creates a full-viewport containerpointer-events: noneallows backdrop clicks to pass through while child elements (like.app-switcherwithpointer-events: all) remain interactivez-indexensures proper stacking orderThis structure effectively coordinates with the
position: absoluteapp-switcher to create the launchpad overlay behavior.
3-6: The.modal-backdropopacity is correctly set to0as the initial state. The backdrop properly animates to a visible state via the Angular animation trigger:
- Initial state (SCSS):
opacity: 0- Animation trigger (TypeScript):
animationTrigger('backdrop', [state('show', style({'opacity': '1'})), transition('* <=> *', [animate('0.15s linear')])])- Template binding:
[@backdrop]="'show'"applies the animation when the backdrop is shown- Conditional rendering: The backdrop is only rendered when
openDropdownCount() || launchpadOpen()is trueThis is a standard Angular animation pattern: CSS provides the baseline state, and the animation trigger handles the transition to the visible state. The 0.15s linear transition ensures smooth visibility changes. Tests confirm the backdrop appears correctly when navigation or action items are opened.
Likely an incorrect or invalid review comment.
projects/element-ng/application-header/launchpad/si-launchpad-app.component.ts (1)
48-48: LGTM! Formatting improvement enhances readability.The blank line logically separates the
actionproperty from the icon-related properties (iconUrl,iconClass), improving code organization.projects/element-ng/application-header/launchpad/si-launchpad.spec.ts (1)
115-115: LGTM! Test expectation updated to match new UI label.The change from 'Favorite apps' to 'Favorites' aligns with the broader launchpad UI refresh and is consistent with the harness update in
si-launchpad.harness.ts.projects/element-ng/application-header/si-application-header.component.html (2)
56-60: LGTM! Animation structure supports responsive launchpad behavior.The new
launchpad-containerwrapper with the[@expand]animation trigger enables smooth entry/exit transitions for the launchpad, aligning with the PR's responsive design objectives.
67-67: LGTM! Backdrop animation correctly simplified.The static
'show'binding is appropriate since the element is conditionally rendered with@if. Thevoid <=> *transition handles the leave animation automatically.projects/element-ng/application-header/testing/si-launchpad.harness.ts (1)
20-20: LGTM! Harness updated to match new Favorites label.The change from 'Favorite apps' to 'Favorites' is consistent with the test expectation update in
si-launchpad.spec.tsand supports the launchpad UI refresh.playwright/e2e/element-examples/si-launchpad.spec.ts (1)
16-16: No action needed—'Fischbach' app exists and is properly configured in the example data.The app is defined in the launchpad example data with
favorite: trueand is included in the businessApps category filter, confirming it's available for the test interaction.src/app/examples/si-application-header/si-launchpad.html (3)
20-39: LGTM: Well-structured form switch with proper accessibility.The "Enable Favorites" toggle has good accessibility with proper label association, descriptive text, and semantic HTML. The layout and styling are clean.
42-63: LGTM: Well-structured form switch with proper accessibility.The "Enable Categories" toggle follows the same accessible pattern as the favorites toggle with proper label association and descriptive text.
82-87: LGTM: Signal-based bindings align with reactive approach.The migration from static property bindings to function-call bindings (
apps(),enableFavorites()) correctly implements Angular's signal-based reactivity pattern.projects/element-ng/application-header/si-application-header.component.ts (1)
51-80: Verify animation translateY value across different viewport sizes.The animations look well-structured, but the
translateY(-120px)value in the expand animation is quite large. Please ensure this works correctly across different viewport sizes and doesn't cause the launchpad to appear off-screen or create layout issues on smaller devices.Consider testing the animation behavior at various breakpoints (mobile, tablet, desktop) to ensure the 120px translation is appropriate for all screen sizes.
projects/element-ng/application-header/launchpad/si-launchpad-app.component.html (1)
6-18: Spacing between App name and System name is correct.The
mt-2class applies 4px spacing (spacing-2) as required, confirmed by the spacer scale definition wherespacing-2 = $spacer * 0.25 = 16px * 0.25 = 4px.projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss (2)
20-32: Good separation of active and hover states.The active state correctly uses
base-1-selected(line 27) while hover usesbase-1-hover(line 31), addressing the PR feedback about using the correct color token for selected items.
77-102: Responsive layout implementation looks solid.The media query correctly switches to row layout at the
mdbreakpoint, with appropriate adjustments for padding, margins, and positioning. The:has(.favorite-icon)selector for conditional padding is a clean approach.src/app/examples/si-application-header/si-launchpad.ts (2)
163-165: Immutable update pattern correctly implemented.The
updateFavoritemethod now uses the immutablemapapproach instead of direct mutation, which is cleaner and more predictable for signal-based state management.
121-161: Categorization logic is clear for demo purposes.The computed
appssignal handles both flat and categorized app lists based on theenableCategoriesflag. The hardcoded name matching for category assignment is acceptable for example code demonstrating the feature.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html (3)
10-25: Well-structured header with proper accessibility.The new
apps-headerblock cleanly groups the title, optional subtitle, and close button. The close button has a properaria-labelfor accessibility, and the icon component automatically handlesaria-hiddeninternally.
63-63: Keyboard navigation binding is correctly implemented.Passing
$event.currentTargetensures the handler receives the element that has the event listener attached (the<a>element), which is the correct reference for navigation calculations.Also applies to: 82-82
30-40: Category title margins create proper visual hierarchy.The conditional classes
[class.mt-4]="first"and[class.mt-8]="!first"provide appropriate spacing: less margin for the first category (following the header) and more margin between subsequent categories.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts (3)
61-64: Title text updated to align with iX design.The default title changed from "Launchpad" to "Switch to application" as noted in the PR objectives. Per the PR comments, there's ongoing discussion about "application" vs "app" wording - this may need adjustment based on the final decision.
163-201: Solid keyboard navigation implementation with RTL support.The
handleAppsNavigationmethod correctly:
- Uses
correctKeyRTLto swap arrow keys for RTL layouts- Validates the key and element before processing
- Uses
:scope >selector for proper isolation (addresses past feedback)- Delegates vertical navigation to the specialized helper
The horizontal wraparound using modulo is clean and intuitive.
203-276: The method already has comprehensive unit test coverage insi-launchpad-navigation.spec.ts. Tests cover single-column navigation, multi-row grid navigation, edge cases (first/last row wrapping, single row), and tolerance handling. No additional tests are needed at this time.Likely an incorrect or invalid review comment.
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
Show resolved
Hide resolved
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.ts
Outdated
Show resolved
Hide resolved
projects/element-ng/application-header/launchpad/si-launchpad-navigation.spec.ts
Outdated
Show resolved
Hide resolved
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html
Outdated
Show resolved
Hide resolved
projects/element-ng/application-header/launchpad/si-launchpad-app.component.html
Show resolved
Hide resolved
projects/element-ng/application-header/launchpad/si-launchpad-app.component.html
Outdated
Show resolved
Hide resolved
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html
Outdated
Show resolved
Hide resolved
projects/element-ng/application-header/si-application-header.component.scss
Outdated
Show resolved
Hide resolved
projects/element-ng/navbar/si-navbar-primary/si-navbar-primary.component.spec.ts
Outdated
Show resolved
Hide resolved
projects/element-ng/application-header/launchpad/si-launchpad-app.component.scss
Show resolved
Hide resolved
9869794 to
a0a35ce
Compare
429dc5f to
a391a6e
Compare
a391a6e to
cd87546
Compare
projects/element-ng/application-header/launchpad/si-launchpad-app.component.ts
Outdated
Show resolved
Hide resolved
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss
Outdated
Show resolved
Hide resolved
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.scss
Outdated
Show resolved
Hide resolved
b1d6a23 to
b804ce9
Compare
Align the launchpad to match iX/Element styling, improve responsiveness, animations and accessibility. NOTE: The `subtitleText` input no longer shows "Access all your apps" by default. To maintain the previous behavior, explicitly set the input.
b804ce9 to
3f7c3c3
Compare
spike-rabbit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍


Aligns launchpad style between iX/Element and introduces responsive behavior.
This PR aligns the Launchpad with iX styling, introduces responsive behavior, and adds keyboard navigation. Key points:
UI & Styling
Components & behavior
Tests, examples & tooling
Public/API changes
Review notes / Actionable items from discussion
Scope & risk: broad UI, behavior, snapshot and API changes — Estimated review effort: Medium–High. PR includes contributor checklist confirmation and is labeled breaking-changes.