feat(password-toggle): support form validation for si-password-toggle #1239
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughSiPasswordToggleComponent now integrates with Angular forms: it captures a NgControl via Sequence Diagram(s)mermaid Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (8)
🧰 Additional context used🧠 Learnings (19)📓 Common learnings📚 Learning: 2025-12-16T16:19:17.950ZApplied to files:
📚 Learning: 2025-12-22T13:04:20.883ZApplied to files:
📚 Learning: 2025-12-09T14:32:34.036ZApplied to files:
📚 Learning: 2025-12-11T14:44:11.278ZApplied to files:
📚 Learning: 2025-12-04T11:54:31.132ZApplied to files:
📚 Learning: 2025-11-18T12:37:30.510ZApplied to files:
📚 Learning: 2025-12-17T04:34:55.597ZApplied to files:
📚 Learning: 2025-12-09T14:33:54.441ZApplied to files:
📚 Learning: 2025-12-17T04:34:59.804ZApplied to files:
📚 Learning: 2025-12-08T11:25:20.861ZApplied to files:
📚 Learning: 2025-12-11T10:09:01.564ZApplied to files:
📚 Learning: 2025-12-08T11:25:51.584ZApplied to files:
📚 Learning: 2025-12-08T11:24:45.272ZApplied to files:
📚 Learning: 2025-12-15T07:17:06.981ZApplied to files:
📚 Learning: 2025-12-15T07:16:32.082ZApplied to files:
📚 Learning: 2025-12-15T07:16:53.762ZApplied to files:
📚 Learning: 2025-12-01T14:12:11.111ZApplied to files:
📚 Learning: 2025-12-15T10:05:59.100ZApplied to files:
🧬 Code graph analysis (1)projects/element-ng/password-toggle/si-password-toggle.component.spec.ts (1)
⏰ 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)
🔇 Additional comments (11)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (6)
playwright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--invalid-password-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--invalid-password-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--valid-password-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--valid-password-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--visibility-toggle-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--visibility-toggle-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (8)
api-goldens/element-ng/password-toggle/index.api.mdplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--invalid-password.yamlplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--valid-password.yamlplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--visibility-toggle.yamlprojects/element-ng/password-toggle/si-password-toggle.component.spec.tsprojects/element-ng/password-toggle/si-password-toggle.component.tssrc/app/examples/input-fields/password.htmlsrc/app/examples/input-fields/password.ts
🧰 Additional context used
🧠 Learnings (13)
📚 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/input-fields/password.tsprojects/element-ng/password-toggle/si-password-toggle.component.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/input-fields/password.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:
src/app/examples/input-fields/password.tsprojects/element-ng/password-toggle/si-password-toggle.component.tsprojects/element-ng/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.component.ts
📚 Learning: 2025-11-18T12:37:30.510Z
Learnt from: Killusions
Repo: siemens/element PR: 1040
File: projects/element-ng/chat-messages/si-chat-input.component.ts:338-349
Timestamp: 2025-11-18T12:37:30.510Z
Learning: In projects/element-ng/chat-messages/si-chat-input.component.ts, the interrupt behavior is intentionally different for button clicks vs Enter key: the button can emit interrupt even without content, but pressing Enter requires content or attachments (canSend() must be true) to emit interrupt.
Applied to files:
projects/element-ng/password-toggle/si-password-toggle.component.ts
📚 Learning: 2025-12-17T04:34:59.804Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:59.804Z
Learning: In zoneless Angular with OnPush change detection, explicit `markForCheck()` calls are only required for asynchronous operations (like setTimeout, setInterval, Promises, Observables). Event handlers triggered by Angular event bindings (click, input, etc.) automatically trigger change detection and do not require explicit `markForCheck()` calls.
Applied to files:
api-goldens/element-ng/password-toggle/index.api.md
📚 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:
api-goldens/element-ng/password-toggle/index.api.mdprojects/element-ng/password-toggle/si-password-toggle.component.spec.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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.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/password-strength.spec.ts-snapshots/input-fields--password--valid-password.yamlplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--invalid-password.yamlplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--visibility-toggle.yaml
⏰ 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 (9)
projects/element-ng/password-toggle/si-password-toggle.component.ts (2)
67-70: Good implementation of validation state tracking.The
ngDoCheckimplementation correctly checks bothtouchedandinvalidstates to determine when to show validation messages. This is the appropriate condition for displaying error feedback to users.
23-23: Use ofcontentChild(NgControl)is appropriate and correctly implemented.The pattern consistently queries for form control directives in projected content and safely handles the undefined case through optional chaining. Validation classes (
ng-invalid,ng-touched) are conditionally applied only when the control exists and meets the validation state conditions. When form directives are not applied to the projected input element,ngControl()returns undefined and validation classes are not applied—which is the correct behavior.projects/element-ng/password-toggle/si-password-toggle.component.spec.ts (1)
29-43: Good test host component for form validation.The
FormHostComponentcorrectly sets up a reactive form with:
updateOn: 'blur'to trigger validation on blur eventsValidators.requiredto test invalid stateThis provides a solid foundation for testing form control integration.
src/app/examples/input-fields/password.ts (1)
7-7: LGTM! Clean addition of form item component.The import and registration of
SiFormItemComponentcorrectly supports the template changes that wrap the password toggle in a form item for better form integration.Also applies to: 18-18
playwright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--valid-password.yaml (1)
1-11: Auto-generated snapshot file - no review needed.This is an auto-generated Playwright snapshot file that captures the accessibility tree for testing purposes. No manual review is required.
playwright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--visibility-toggle.yaml (1)
1-11: Auto-generated snapshot file - no review needed.This is an auto-generated Playwright snapshot file that captures the accessibility tree for testing purposes. No manual review is required.
playwright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--invalid-password.yaml (1)
1-11: Auto-generated snapshot file - no review needed.This is an auto-generated Playwright snapshot file that captures the accessibility tree for testing purposes. No manual review is required.
src/app/examples/input-fields/password.html (1)
4-16: LGTM! Proper form integration with si-form-item.The password toggle is now correctly wrapped in
si-form-itemwith appropriate form directives:
labelattribute provides accessible labeling (asterisk for required fields is likely auto-added by si-form-item)requiredattribute enables validationnameattribute registers the control with the formngModelprovides two-way binding with a demo initial valueThis properly demonstrates the new form validation capabilities.
api-goldens/element-ng/password-toggle/index.api.md (1)
8-9: API documentation correctly reflects component changes.The API golden file accurately documents the new public API surface:
DoCheckinterface implementationNgControlusage for form integrationngDoCheck()lifecycle hookThis file is auto-generated by API Extractor and correctly reflects the component's public API changes.
Also applies to: 13-13, 16-17, 22-22
projects/element-ng/password-toggle/si-password-toggle.component.spec.ts
Show resolved
Hide resolved
projects/element-ng/password-toggle/si-password-toggle.component.ts
Outdated
Show resolved
Hide resolved
projects/element-ng/password-toggle/si-password-toggle.component.ts
Outdated
Show resolved
Hide resolved
|
Documentation. Coverage Reports: |
spliffone
force-pushed
the
feat/support-password-toggle-form-usage
branch
from
8ab315c to
a16c484
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (6)
playwright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--invalid-password-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--invalid-password-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--valid-password-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--valid-password-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--visibility-toggle-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--visibility-toggle-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (8)
api-goldens/element-ng/password-toggle/index.api.mdplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--invalid-password.yamlplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--valid-password.yamlplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--visibility-toggle.yamlprojects/element-ng/password-toggle/si-password-toggle.component.spec.tsprojects/element-ng/password-toggle/si-password-toggle.component.tssrc/app/examples/input-fields/password.htmlsrc/app/examples/input-fields/password.ts
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:59.804Z
Learning: In zoneless Angular with OnPush change detection, explicit `markForCheck()` calls are only required for asynchronous operations (like setTimeout, setInterval, Promises, Observables). Event handlers triggered by Angular event bindings (click, input, etc.) automatically trigger change detection and do not require explicit `markForCheck()` calls.
📚 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/input-fields/password.tsprojects/element-ng/password-toggle/si-password-toggle.component.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/input-fields/password.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:
src/app/examples/input-fields/password.tsprojects/element-ng/password-toggle/si-password-toggle.component.tsprojects/element-ng/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.component.ts
📚 Learning: 2025-11-18T12:37:30.510Z
Learnt from: Killusions
Repo: siemens/element PR: 1040
File: projects/element-ng/chat-messages/si-chat-input.component.ts:338-349
Timestamp: 2025-11-18T12:37:30.510Z
Learning: In projects/element-ng/chat-messages/si-chat-input.component.ts, the interrupt behavior is intentionally different for button clicks vs Enter key: the button can emit interrupt even without content, but pressing Enter requires content or attachments (canSend() must be true) to emit interrupt.
Applied to files:
projects/element-ng/password-toggle/si-password-toggle.component.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/password-strength.spec.ts-snapshots/input-fields--password--valid-password.yamlplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--invalid-password.yamlplaywright/snapshots/password-strength.spec.ts-snapshots/input-fields--password--visibility-toggle.yaml
📚 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/password-toggle/si-password-toggle.component.spec.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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.component.spec.tsapi-goldens/element-ng/password-toggle/index.api.md
📚 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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.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/password-toggle/si-password-toggle.component.spec.ts
📚 Learning: 2025-12-17T04:34:59.804Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:59.804Z
Learning: In zoneless Angular with OnPush change detection, explicit `markForCheck()` calls are only required for asynchronous operations (like setTimeout, setInterval, Promises, Observables). Event handlers triggered by Angular event bindings (click, input, etc.) automatically trigger change detection and do not require explicit `markForCheck()` calls.
Applied to files:
api-goldens/element-ng/password-toggle/index.api.md
🧬 Code graph analysis (1)
projects/element-ng/password-toggle/si-password-toggle.component.spec.ts (2)
projects/element-ng/password-toggle/si-password-toggle.component.ts (1)
Component(10-76)src/app/examples/input-fields/password.ts (1)
Component(15-30)
⏰ 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 (9)
projects/element-ng/password-toggle/si-password-toggle.component.ts (3)
5-6: LGTM! Imports added for form integration.The imports for
contentChild,DoCheck, andNgControlare correctly added to support form validation integration.
22-23: LGTM! DoCheck implementation and NgControl integration.The component correctly implements
DoCheckand usescontentChildto access theNgControlfor form integration.
58-70: LGTM! Validation state signals and synchronization logic.The
isTouchedandisInvalidsignals are correctly declared, and thengDoCheck()implementation properly synchronizes them with the underlying form control's state. The logic correctly handles the case whenngControlorcontrolmight be undefined.projects/element-ng/password-toggle/si-password-toggle.component.spec.ts (3)
7-13: LGTM! Reactive forms imports added.The necessary imports for reactive forms testing (
FormControl,FormGroup,ReactiveFormsModule,Validators) are correctly added.
29-43: LGTM! FormHostComponent configured correctly.The
FormHostComponentis properly set up with:
- Reactive forms integration via
FormGroupandFormControlupdateOn: 'blur'to defer validation until blur eventValidators.requiredto test validation state
90-109: Test structure is correct but will fail due to component bug.The test is well-structured and correctly:
- Creates a dedicated
formFixturewith proper type- Uses
formFixture.detectChanges()(not the parentfixture)- Tests that
ng-touchedandng-invalidclasses are applied after blur on an empty required fieldHowever, this test will fail because the host bindings in the component (lines 17-18 of
si-password-toggle.component.ts) are swapped. Once that critical issue is fixed, this test should pass.api-goldens/element-ng/password-toggle/index.api.md (1)
8-9: LGTM! API surface correctly updated.The public API changes accurately reflect the form integration:
DoCheckandNgControlimports added- Component now implements
DoCheckinterfacengDoCheck(): voidmethod exposed in public API- Component metadata includes
ngControlin content/query arrayAlso applies to: 13-13, 17-17, 22-22
src/app/examples/input-fields/password.ts (1)
7-7: LGTM! Form item component added for example.The
SiFormItemComponentis correctly imported and registered to support the updated example template that demonstrates form integration with the password toggle.Also applies to: 18-18
src/app/examples/input-fields/password.html (1)
4-16: LGTM! Example updated to demonstrate form integration.The password toggle is now properly wrapped in
si-form-itemwith:
- Label provided via the
labelattribute- Form attributes (
required,name,ngModel) for validation and binding- Proper form control structure demonstrating the new validation capabilities
This effectively showcases the form validation support added to the component.
projects/element-ng/password-toggle/si-password-toggle.component.ts
Outdated
Show resolved
Hide resolved
spliffone
force-pushed
the
feat/support-password-toggle-form-usage
branch
from
a16c484 to
94ad766
Compare
spliffone
force-pushed
the
feat/support-password-toggle-form-usage
branch
from
94ad766 to
a2c22a0
Compare
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds form validation support to the si-password-toggle component so it integrates with Angular reactive and template-driven forms.
Key changes
Impact
si-password-toggle now participates in Angular forms, reflecting ng-touched/ng-invalid states when bound to a form control.