Angular 19 - #34
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughUpgrades the app to Angular 19 and PrimeNG 19, updates CLI targets and package versions, wires PrimeNG theming in ChangesAngular 19 & PrimeNG 19 Upgrade
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@main/http_server/forge-os/src/app/components/pool/pool.component.scss`:
- Around line 167-171: The focus styles on the tab selector in
pool.component.scss remove all visible keyboard indication for .p-tab. Update
the &:focus and &:focus-visible rules so the tab still has a clear focus
indicator, keeping a visible outline or box-shadow rather than suppressing both,
and ensure the change is applied in the same tab styling block.
In
`@main/http_server/forge-os/src/app/components/settings/settings.component.html`:
- Around line 60-61: The PrimeNG file upload in settings.component.html is using
an incomplete chooseIcon value, so update the p-fileUpload chooseIcon binding
from the icon class fragment to the full PrimeIcons class string. Locate the
websiteUpload p-fileUpload markup and make sure both browse/upload icon usages
follow the same pi pi-plus format so the icon renders correctly.
In
`@main/http_server/forge-os/src/app/layout/styles/theme/theme-base/_components.scss`:
- Line 5: The layer placement for theme base component styles is causing the
cascade to favor base rules over extension overrides. Review the `@layer
forge-os` wrapping in `theme-base/_components.scss` together with the layer
order in `styles.scss` and the extension layer usage in
`themes/vela/_extensions.scss`, then adjust the layer declarations/order so
extension styles can take precedence over the base theme as intended.
In
`@main/http_server/forge-os/src/app/layout/styles/theme/theme-base/components/input/_dropdown.scss`:
- Line 78: The argumentless mixin call in the dropdown input styles is using
parentheses and triggers the stylelint rule. Update the focused-input mixin
invocation in the relevant SCSS partial to use the no-argument mixin syntax, and
check for any similar argumentless mixin calls in nearby component styles to
keep them consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6a0a3529-a2b8-4b06-ae8d-388f383feab0
⛔ Files ignored due to path filters (2)
main/http_server/forge-os/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (35)
main/http_server/forge-os/angular.jsonmain/http_server/forge-os/package.jsonmain/http_server/forge-os/src/app/app.component.tsmain/http_server/forge-os/src/app/app.module.tsmain/http_server/forge-os/src/app/components/edit/edit.component.tsmain/http_server/forge-os/src/app/components/home/home.component.tsmain/http_server/forge-os/src/app/components/loading/loading.component.tsmain/http_server/forge-os/src/app/components/logs/logs.component.tsmain/http_server/forge-os/src/app/components/modal/modal.component.tsmain/http_server/forge-os/src/app/components/network-edit/network.edit.component.scssmain/http_server/forge-os/src/app/components/network-edit/network.edit.component.tsmain/http_server/forge-os/src/app/components/network/network.component.tsmain/http_server/forge-os/src/app/components/pool/pool.component.scssmain/http_server/forge-os/src/app/components/pool/pool.component.tsmain/http_server/forge-os/src/app/components/settings/settings.component.htmlmain/http_server/forge-os/src/app/components/settings/settings.component.scssmain/http_server/forge-os/src/app/components/settings/settings.component.tsmain/http_server/forge-os/src/app/components/swarm/swarm.component.tsmain/http_server/forge-os/src/app/layout/app.footer.component.tsmain/http_server/forge-os/src/app/layout/app.layout.component.tsmain/http_server/forge-os/src/app/layout/app.menu.component.tsmain/http_server/forge-os/src/app/layout/app.menuitem.component.tsmain/http_server/forge-os/src/app/layout/app.sidebar.component.tsmain/http_server/forge-os/src/app/layout/app.topbar.component.tsmain/http_server/forge-os/src/app/layout/styles/theme/theme-base/_components.scssmain/http_server/forge-os/src/app/layout/styles/theme/theme-base/components/button/_button.scssmain/http_server/forge-os/src/app/layout/styles/theme/theme-base/components/input/_dropdown.scssmain/http_server/forge-os/src/app/pipes/ansi.pipe.tsmain/http_server/forge-os/src/app/pipes/date-ago.pipe.tsmain/http_server/forge-os/src/app/pipes/diff-suffix.pipe.tsmain/http_server/forge-os/src/app/pipes/hash-suffix.pipe.tsmain/http_server/forge-os/src/app/pipes/truncate-middle.pipe.tsmain/http_server/forge-os/src/app/prime-ng.module.tsmain/http_server/forge-os/src/app/services/dialog.service.tsmain/http_server/forge-os/src/styles.scss
| &:focus, | ||
| &:focus-visible { | ||
| box-shadow: none; | ||
| outline: none; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore a visible keyboard focus state on tabs.
Line 167-Line 171 remove both outline and box-shadow, leaving no visible focus indicator for keyboard navigation on .p-tab.
Suggested fix
&:focus,
&:focus-visible {
- box-shadow: none;
- outline: none;
+ outline: 2px solid `#6366f1`;
+ outline-offset: 2px;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| &:focus, | |
| &:focus-visible { | |
| box-shadow: none; | |
| outline: none; | |
| } | |
| &:focus, | |
| &:focus-visible { | |
| outline: 2px solid `#6366f1`; | |
| outline-offset: 2px; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@main/http_server/forge-os/src/app/components/pool/pool.component.scss` around
lines 167 - 171, The focus styles on the tab selector in pool.component.scss
remove all visible keyboard indication for .p-tab. Update the &:focus and
&:focus-visible rules so the tab still has a clear focus indicator, keeping a
visible outline or box-shadow rather than suppressing both, and ensure the
change is applied in the same tab styling block.
| <p-fileUpload #websiteUpload [customUpload]="true" mode="basic" accept=".bin" (uploadHandler)="otaWWWUpdate($event)" | ||
| [auto]="true" chooseLabel="Browse" [disabled]="isUpdating"></p-fileUpload> | ||
| [auto]="true" chooseLabel="Browse" chooseIcon="pi-plus" [disabled]="isUpdating"></p-fileUpload> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
PrimeNG 19 FileUpload chooseIconinput: should the value bepi-plusor full classes likepi pi-plus? Please cite the official docs/examples.
💡 Result:
For the PrimeNG 19 FileUpload component, the chooseIcon property requires the full class string, such as pi pi-plus [1]. According to the official PrimeNG documentation, the chooseIcon attribute expects a string that defines the icon class [1]. As demonstrated in the component's official usage examples, you must provide both the base pi class and the specific icon class (e.g., chooseIcon="pi pi-upload") [1]. This aligns with the standard usage of the PrimeIcons library, which defines icons using the pi pi-{icon} syntax [2]. Providing only the specific icon class (e.g., pi-plus) will typically result in the icon failing to render correctly because the base pi class is required to apply the necessary icon styling [2]. For greater customization beyond standard PrimeIcons, you can use the chooseicon template, which allows you to insert custom elements like images, SVG, or other icon libraries [3][4]. Top results: [1][2]
Citations:
- 1: https://v19.primeng.org/fileupload
- 2: https://v19.primeng.org/icons
- 3: https://stackoverflow.com/questions/78093398/p-fileupload-change-default-icon-of-the-control-for-multiple-controls
- 4: https://v19.primeng.org/customicons
Correct PrimeNG chooseIcon class format.
The chooseIcon property expects a full PrimeIcons class string (base class + icon class). The current value pi-plus is missing the required base pi class, which will prevent the icon from rendering. Update both instances to use pi pi-plus.
Lines to update
<p-fileUpload `#websiteUpload` [customUpload]="true" mode="basic" accept=".bin" (uploadHandler)="otaWWWUpdate($event)"
[auto]="true" chooseLabel="Browse" chooseIcon="pi pi-plus" [disabled]="isUpdating"></p-fileUpload>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@main/http_server/forge-os/src/app/components/settings/settings.component.html`
around lines 60 - 61, The PrimeNG file upload in settings.component.html is
using an incomplete chooseIcon value, so update the p-fileUpload chooseIcon
binding from the icon class fragment to the full PrimeIcons class string. Locate
the websiteUpload p-fileUpload markup and make sure both browse/upload icon
usages follow the same pi pi-plus format so the icon renders correctly.
|
|
||
|
|
||
| @layer primeng { | ||
| @layer forge-os { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show global layer order and changed layer target
rg -n '`@layer`' main/http_server/forge-os/src/styles.scss \
main/http_server/forge-os/src/app/layout/styles/theme/theme-base/_components.scss
# Enumerate theme files that still emit `@layer` primeng / forge-os
fd -a --extension scss . main/http_server/forge-os/src/app/layout/styles/theme \
| xargs rg -n '`@layer`\s+(primeng|forge-os)'Repository: WantClue/forge-os
Length of output: 569
Verify CSS cascade priority for extension overrides.
The current layer order in styles.scss is @layer primeng, forge-os. Since theme-base/_components.scss is now wrapped in @layer forge-os while extensions (e.g., themes/vela/_extensions.scss) remain in @layer primeng, base component styles will override extension customizations.
This inverts the standard theming model where extensions should take precedence. Please confirm this is intentional or adjust the layer declarations to ensure extensions can override the base.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@main/http_server/forge-os/src/app/layout/styles/theme/theme-base/_components.scss`
at line 5, The layer placement for theme base component styles is causing the
cascade to favor base rules over extension overrides. Review the `@layer
forge-os` wrapping in `theme-base/_components.scss` together with the layer
order in `styles.scss` and the extension layer usage in
`themes/vela/_extensions.scss`, then adjust the layer declarations/order so
extension styles can take precedence over the base theme as intended.
| } | ||
|
|
||
| &:not(.p-disabled).p-focus { | ||
| @include focused-input(); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
cat -n main/http_server/forge-os/src/app/layout/styles/theme/theme-base/components/input/_dropdown.scss | sed -n '70,85p'Repository: WantClue/forge-os
Length of output: 600
🏁 Script executed:
cat -n main/http_server/forge-os/src/app/layout/styles/theme/theme-base/components/input/_dropdown.scss | sed -n '105,120p'Repository: WantClue/forge-os
Length of output: 601
🏁 Script executed:
cat -n main/http_server/forge-os/src/app/layout/styles/theme/theme-base/components/input/_dropdown.scss | sed -n '310,325p'Repository: WantClue/forge-os
Length of output: 372
🏁 Script executed:
find main/http_server/forge-os -name ".stylelintrc*" -o -name "stylelint.config.*" -o -name "package.json" 2>/dev/null | xargs -I{} sh -c 'echo "=== {} ==="; cat "{}"'Repository: WantClue/forge-os
Length of output: 1949
Fix stylelint failures for argumentless mixin calls.
These lines use parentheses on argumentless mixins and trigger the current stylelint rule, which will fail CI.
Suggested patch
- `@include` focused-input();
+ `@include` focused-input;
@@
- `@include` invalid-input();
+ `@include` invalid-input;
@@
- `@include` invalid-input();
+ `@include` invalid-input;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @include focused-input(); | |
| `@include` focused-input; |
🧰 Tools
🪛 Stylelint (17.13.0)
[error] 78-78: Unexpected parentheses in argumentless mixin "focused-input" call (scss/at-mixin-argumentless-call-parentheses)
(scss/at-mixin-argumentless-call-parentheses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@main/http_server/forge-os/src/app/layout/styles/theme/theme-base/components/input/_dropdown.scss`
at line 78, The argumentless mixin call in the dropdown input styles is using
parentheses and triggers the stylelint rule. Update the focused-input mixin
invocation in the relevant SCSS partial to use the no-argument mixin syntax, and
check for any similar argumentless mixin calls in nearby component styles to
keep them consistent.
Source: Linters/SAST tools
1 participant
Moves over to angular 19
Summary by CodeRabbit
New Features
Bug Fixes
Chores