Conversation
Update develop branch
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…up-workflow Add Firebase preview channel cleanup workflow
…-tailwind-css Improve landing styles
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…-specular-lighting Add WebGL ball route
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…-specular-lighting Add WebGL ball route
…usar-node-18 Add Node 22 setup in Firebase workflows
…l-landing Auto-load personal section
Add a root CodeRabbit config that enables automatic and incremental reviews for PRs with develop as the base branch.
Regenerate the lockfile for npm 10 compatibility and run Node.js CI on Node 22 so the GitHub Actions jobs can complete npm ci, test, lint, and build steps.
chore(angular): upgrade app to Angular 21
Remove duplicate HttpClient providers from feature modules and fix the Open Source UC community copy raised in review feedback.
…ents fix(review): address PR 359 follow-up comments
Harden personal info access, localize the new landing copy, and add a safe WebGL fallback with tests.
Tighten cookie cleanup, modernize the Ball spec router setup, and clean up the remaining minor review issues.
Harden the private token input and keep the Ball fallback debuggable without reintroducing noisy WebGL errors.
fix(landing): harden private notes and WebGL fallback
Refresh codex/usar-colores-más-equilibrados-en-el-sitio-web with the latest develop branch.
Update the landing, CV viewer, and shell chrome with the Personal Web Page mission-control concept derived from Stitch project 3048263380749146026.
Split the landing into console panels, ground the content in the live site and CV, and add the private-note/sidebar integration for the new home shell.
Align the navbar, footer, route chrome, experience view, and anchor-driven navigation so the landing and curriculum behave like one console system.
feat(landing): harmonize the shared shell Align the navbar, footer, route chrome, experience view, and anchor-driven navigation so the landing and curriculum behave like one console system.
📝 WalkthroughWalkthroughComprehensive redesign of the landing application introducing a console-styled UI system with lazy-loaded routes, refactored components (home, footer, navbar), new Three.js WebGL ball component, personal-info service with token-based authentication, updated GitHub Actions workflows, and redesigned color palette with CSS custom properties. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Component as PersonalInfoComponent
participant Service as PersonalInfoService
participant Http as HttpClient
participant Cookie as Browser Cookie
User->>Component: Page Load
Note over Component: ngOnInit()
Component->>Cookie: readTokenFromCookie('personal_token')
alt Token Found
Cookie-->>Component: token
Component->>Service: getPersonalInfo(token)
Service->>Http: GET /api/personal-info?token=...
Http-->>Service: PrivateInfo
Service-->>Component: Observable<PrivateInfo>
Component->>Component: personal$ = stream
else Token Not Found
Cookie-->>Component: null
Component->>Component: errorKey = undefined
end
User->>Component: submitToken(input)
Component->>Component: trim & validate
alt Valid Input
Component->>Cookie: persistToken(token)
Component->>Service: getPersonalInfo(token)
Service->>Http: GET /api/personal-info?token=...
Http-->>Service: PrivateInfo
Service-->>Component: Observable<PrivateInfo>
else Empty Input
Component->>Cookie: delete personal_token
Component->>Component: Reset state
end
User->>Component: clearToken()
Component->>Cookie: delete personal_token
Component->>Component: token = '', personal$ = undefined
sequenceDiagram
participant User as User
participant NavBar as NavbarComponent
participant Router as Angular Router
participant InfoComponent as InfoComponent (home)
User->>NavBar: Load Page / Navigate
NavBar->>Router: Inject Router
NavBar->>NavBar: currentPath() = extract router.url
alt Route = / or /#...
NavBar->>NavBar: isHomeRoute() = true
NavBar->>NavBar: currentContextLabel() = 'PROFILE_ACTIVE'
NavBar-->>User: Render "Overview" as active
else Route = /experience/...
NavBar->>NavBar: isExperienceRoute() = true
NavBar->>NavBar: currentContextLabel() = 'CURRICULUM_ACTIVE'
NavBar-->>User: Render "Experience" as active
else Other Route
NavBar->>NavBar: currentContextLabel() = 'ROUTE_ACTIVE'
end
Note over InfoComponent: Console UI Initialization
InfoComponent->>InfoComponent: railPrimaryItems = createConsoleRailPrimaryItems('Overview')
InfoComponent->>InfoComponent: Set active='true' for matching label
InfoComponent-->>User: Render console-left-rail with active item
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (13)
src/styles.sass-37-44 (1)
37-44:⚠️ Potential issue | 🟡 Minor
font-weight: 25appears incorrect.Font weights typically use values from 100-900 in increments of 100. A value of
25is outside the normal range and may not render as expected with most fonts. Did you mean250or400(normal)?🐛 Proposed fix (assuming normal weight intended)
.console - font-weight: 25 + font-weight: 400 font-family:'JetBrains Mono', monospace text-align: left .console h6 - font-weight: 25 + font-weight: 400 font-family: 'JetBrains Mono', monospace🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/styles.sass` around lines 37 - 44, The CSS uses invalid font-weight: 25 in the .console and .console h6 rules; update both occurrences to a valid weight (e.g., 400 for normal text or 250/300 if a lighter numeric weight is required) so the font renders correctly; locate the .console and .console h6 selectors in styles.sass and replace font-weight: 25 with the chosen valid value and confirm JetBrains Mono supports that weight.src/index.html-39-46 (1)
39-46:⚠️ Potential issue | 🟡 MinorUpdate Firebase SDK from 7.18.0 to the latest version (12.10.0).
Firebase JS SDK 7.18.0 (released ~2020) is significantly outdated. The current version is 12.10.0, leaving substantial gaps in security fixes and features. Update Firebase in a follow-up to address this version mismatch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.html` around lines 39 - 46, The HTML includes Firebase SDK script tags pinning version 7.18.0 (firebase-app.js and firebase-analytics.js) which is outdated; update the src attributes to reference the current Firebase JS SDK version 12.10.0 (replace "/__/firebase/7.18.0/..." with "/__/firebase/12.10.0/...") and ensure the consumed init.js is compatible (update or regenerate init.js if your Firebase hosting auto-provides a versioned init, and verify firebase.initializeApp / import usage matches v12 APIs). After updating, run the app and smoke-test analytics/auth/other Firebase features and fix any API changes surfaced by v12.10.0.src/template.css-171-178 (1)
171-178:⚠️ Potential issue | 🟡 Minor
background-coloris overridden by subsequentbackgroundshorthand.The
background-colordeclaration on line 173 is immediately overridden by thebackgroundshorthand on line 174, making it ineffective. Either remove the redundantbackground-coloror move it after the shorthand if you want it as a fallback.🐛 Proposed fix
.call-to-action { position: relative; - background-color: var(--console-bg); background: url('assets/img/bg-masthead.jpg') no-repeat center center; + background-color: var(--console-bg); background-size: cover;Or if
background-coloris intended as a fallback when the image doesn't load:.call-to-action { position: relative; - background-color: var(--console-bg); - background: url('assets/img/bg-masthead.jpg') no-repeat center center; + background: var(--console-bg) url('assets/img/bg-masthead.jpg') no-repeat center center; background-size: cover;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/template.css` around lines 171 - 178, The .call-to-action rule sets background-color then immediately overrides it with the background shorthand; fix by either removing the redundant background-color declaration, moving the background-color after the background shorthand so it acts as a fallback, or incorporate the color into the background shorthand (so .call-to-action retains a fallback color when the image fails to load).src/theme/colors.sass-9-11 (1)
9-11:⚠️ Potential issue | 🟡 MinorInconsistent variable naming:
$secondary-Darkshould use lowercased.The variable
$secondary-Darkuses a capitalDwhile$secondary-lightuses lowercase, breaking naming consistency. The variable has no other usages in the codebase, so the rename is safe.-$secondary-Dark: `#26467d` +$secondary-dark: `#26467d`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/theme/colors.sass` around lines 9 - 11, Rename the inconsistent Sass variable `$secondary-Dark` to use a lowercase d (e.g. `$secondary-dark`) to match the existing `$secondary-light` naming convention; update the variable declaration in the colors.sass snippet and replace any references to `$secondary-Dark` elsewhere (search/replace) so all usages use `$secondary-dark` consistently.src/app/navbar/navbar.component.sass-88-107 (1)
88-107:⚠️ Potential issue | 🟡 MinorAdd explicit
:focus-visiblestates for keyboard navigation.This block styles hover states but not keyboard focus for the same controls (
.console-navbar__github,.console-navbar__toggle,.nav-link,.mobile-nav-link), which can reduce discoverability for non-mouse users.Suggested CSS patch
.console-navbar__github:hover border-color: rgba(173, 198, 255, 0.36) background: rgba(26, 36, 61, 0.95) color: `#fff` + +.console-navbar__github:focus-visible + border-color: rgba(173, 198, 255, 0.36) + background: rgba(26, 36, 61, 0.95) + color: `#fff` + outline: 2px solid rgba(173, 198, 255, 0.45) + outline-offset: 2px .console-navbar__toggle:hover border-color: rgba(173, 198, 255, 0.36) background: rgba(26, 36, 61, 0.95) + +.console-navbar__toggle:focus-visible + border-color: rgba(173, 198, 255, 0.36) + background: rgba(26, 36, 61, 0.95) + outline: 2px solid rgba(173, 198, 255, 0.45) + outline-offset: 2px .nav-link:hover color: `#fff` + +.nav-link:focus-visible + color: `#fff` + text-decoration: underline + text-decoration-thickness: 2px + text-underline-offset: 8px .mobile-nav-link:hover border-color: rgba(173, 198, 255, 0.4) background: `#1a243d` color: `#fff` + +.mobile-nav-link:focus-visible + border-color: rgba(173, 198, 255, 0.4) + background: `#1a243d` + color: `#fff` + outline: 2px solid rgba(173, 198, 255, 0.45) + outline-offset: 2pxAlso applies to: 135-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/navbar/navbar.component.sass` around lines 88 - 107, Add explicit keyboard focus styles by adding matching :focus-visible rules alongside the existing hover selectors so keyboard users see the same visual cues; update selectors .console-navbar__github, .console-navbar__toggle, .nav-link, and .mobile-nav-link to include :focus-visible (e.g. .console-navbar__github:focus-visible, .console-navbar__toggle:focus-visible, .nav-link:focus-visible, .mobile-nav-link:focus-visible) and apply the same border-color, background and color values used in their :hover rules, ensuring transitions remain consistent and include focus-visible states for accessibility.src/app/home/info/info.component.spec.ts-14-14 (1)
14-14:⚠️ Potential issue | 🟡 MinorTest suite name mismatch.
The
describeblock is named'HeaderComponent'but the test file is forInfoComponent. This appears to be a copy-paste artifact and should be corrected for clarity.🐛 Proposed fix
-describe('HeaderComponent', () => { +describe('InfoComponent', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/info/info.component.spec.ts` at line 14, The test suite name is incorrect: change the describe string from 'HeaderComponent' to 'InfoComponent' so the suite matches the tested component (look for the describe('HeaderComponent', ...) line in the spec and update it to describe('InfoComponent', ...)); no other test logic should change.src/app/home/info/console-pipeline-panel.component.html-19-24 (1)
19-24:⚠️ Potential issue | 🟡 MinorAdd accessibility attributes to the progress bar.
The progress bar is purely visual. Screen readers won't convey progress information without appropriate ARIA attributes.
♿ Proposed fix
<div class="console-pipelines__track"> <div class="console-pipelines__fill" [style.width.%]="stage.progress" + role="progressbar" + [attr.aria-valuenow]="stage.progress" + aria-valuemin="0" + aria-valuemax="100" + [attr.aria-label]="stage.label + ' progress'" ></div> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/info/console-pipeline-panel.component.html` around lines 19 - 24, The progress bar is visual only; add ARIA attributes to the element with class "console-pipelines__fill" (or its container "console-pipelines__track") so screen readers can announce progress: give it role="progressbar", aria-valuemin="0", aria-valuemax="100", and bind aria-valuenow to the existing stage.progress; also include an accessible label via aria-label or aria-labelledby (e.g., using the stage name or a nearby heading) to describe what the progress represents. Ensure the existing [style.width.%]="stage.progress" remains unchanged.src/app/home/info/console-left-rail.component.html-11-14 (1)
11-14:⚠️ Potential issue | 🟡 MinorAccessibility: External links should indicate they open in a new window.
Screen reader users benefit from knowing a link will open in a new tab/window. Consider adding
aria-labelor a visually-hidden indicator.♿ Proposed fix
<a [href]="item.href" target="_blank" rel="noopener noreferrer" class="console-rail__link" [class.console-rail__link--active]="item.active" + [attr.aria-label]="item.label + ' (opens in new tab)'" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/info/console-left-rail.component.html` around lines 11 - 14, Update the anchor in console-left-rail.component.html to communicate that it opens a new window by adding an accessible label or hidden text: for example set aria-label to a descriptive string using the item's title (e.g. aria-label="{{item.title}} (opens in a new window)") or include a visually-hidden <span> inside the <a> (class="sr-only") with the text "(opens in a new window)"; ensure the element that uses [href]="item.href" and target="_blank" retains rel="noopener noreferrer" and that the label/span references item.title so screen readers get context.package.json-37-37 (1)
37-37:⚠️ Potential issue | 🟡 MinorMove
pre-committodevDependencies.The
pre-commitpackage is a development tool and should not be independencies. Production builds don't need pre-commit hooks.♻️ Proposed fix
Move
"pre-commit": "^1.2.2"fromdependenciestodevDependencies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 37, Move the "pre-commit" package entry out of the package.json "dependencies" object and add it to "devDependencies" instead: remove the line '"pre-commit": "^1.2.2",' from the dependencies section and insert the same entry under devDependencies so that "pre-commit" is only listed in the devDependencies key.src/app/services/personal-info.service.spec.ts-18-30 (1)
18-30:⚠️ Potential issue | 🟡 MinorEnvironment mutation timing may cause test flakiness.
The environment is mutated on line 29, but the service is injected on line 27. If
PersonalInfoServicereadsenvironment.personalInfoUrlduring construction (e.g., in the constructor), the tests will use the original value rather than/info. Move the mutation beforeTestBed.inject(PersonalInfoService).🐛 Proposed fix
beforeEach(() => { originalPersonalInfoUrl = environment.personalInfoUrl; + (environment as any).personalInfoUrl = '/info'; TestBed.configureTestingModule({ imports: [], providers: [ provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting(), ], }); service = TestBed.inject(PersonalInfoService); httpMock = TestBed.inject(HttpTestingController); - (environment as any).personalInfoUrl = '/info'; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/services/personal-info.service.spec.ts` around lines 18 - 30, Move the environment mutation so the service is constructed with the mocked URL: in the beforeEach block update (environment as any).personalInfoUrl = '/info' before calling TestBed.inject(PersonalInfoService). Ensure you still call TestBed.configureTestingModule(...) and provideHttpClient/provideHttpClientTesting() first, then set environment.personalInfoUrl, then call service = TestBed.inject(PersonalInfoService) and httpMock = TestBed.inject(HttpTestingController) so PersonalInfoService reads the mocked value during construction.src/app/experience/viewer/viewer.component.html-44-73 (1)
44-73:⚠️ Potential issue | 🟡 MinorDrive the toolbar from the real PDF load state.
The success chip always says
Pdf Loaded, andNextstays enabled whiletotalPagesis still unknown. On a slow or failed load, the toolbar reports a ready document before one exists.🛠️ Proposed fix
<div class="experience-viewer__chips"> - <span - class="experience-viewer__chip experience-viewer__chip--success" - > - Pdf Loaded - </span> + <span + class="experience-viewer__chip" + [class.experience-viewer__chip--success]="!!totalPages" + > + {{ totalPages ? 'Pdf Loaded' : 'Loading PDF' }} + </span> <span class="experience-viewer__chip"> Page {{ page }} / {{ totalPages || '?' }} </span> <span class="experience-viewer__chip"> Zoom {{ zoom.toFixed(1) }}x @@ <button (click)="nextPage()" - [disabled]="totalPages && page >= totalPages" + [disabled]="!totalPages || page >= totalPages" class="experience-viewer__button" > {{ 'next' | translate }} </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/experience/viewer/viewer.component.html` around lines 44 - 73, The toolbar currently assumes the PDF is ready; add and use a real loaded state (e.g., a boolean property pdfLoaded or isPdfLoaded set when the PDF load success callback runs and cleared on error/start) and update the template bindings: make the success chip text and class conditional on pdfLoaded, disable the Next/Previous buttons when !pdfLoaded or when totalPages constraints apply, and hide or replace the zoom/page chips with a loading indicator while pdfLoaded is false (use existing symbols page, totalPages, zoom.toFixed, prevPage(), nextPage() to locate where to change the bindings). Ensure the pdfLoaded flag is set in the component’s PDF load success/error handlers so the toolbar reflects actual load state.src/app/services/personal-info.service.ts-33-41 (1)
33-41:⚠️ Potential issue | 🟡 MinorReject non-HTTP endpoints in
resolveUrl().
new URL()accepts non-API schemes too, soisConfigured()can report success for values that will never work with this fetch path. Restrict the accepted protocols tohttp:/https:before enabling the private-notes flow.🛠️ Proposed fix
private resolveUrl(): string | null { const rawUrl = environment.personalInfoUrl?.trim(); if (!rawUrl) { return null; } try { - return new URL(rawUrl, window.location.origin).toString(); + const url = new URL(rawUrl, window.location.origin); + return url.protocol === 'http:' || url.protocol === 'https:' + ? url.toString() + : null; } catch { return null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/services/personal-info.service.ts` around lines 33 - 41, resolveUrl() currently accepts any URL scheme because new URL() allows non-HTTP protocols; update resolveUrl() to validate that the parsed URL.protocol is either "http:" or "https:" and return null for any other protocol so non-HTTP endpoints are rejected (this will ensure isConfigured() only reports success for usable endpoints). Specifically, inside resolveUrl() after creating the URL from rawUrl, check url.protocol and return null unless it equals "http:" or "https:"; keep the try/catch behavior and ensure the function still returns the string form for valid HTTP(S) URLs.src/app/navbar/navbar.component.html-42-48 (1)
42-48:⚠️ Potential issue | 🟡 MinorAdd a safe fallback for the GitHub link.
On Line 42,
html_urlcan beundefinedwhile data loads/fails, leaving the anchor without a usable destination. Add a fallback URL (and label) so the CTA always works.Suggested fix
- [href]="(githubService$ | async)?.html_url" + [href]="(githubService$ | async)?.html_url || 'https://github.com/FarDust/fardust-web'" @@ - <span>{{ (githubService$ | async)?.login }}</span> + <span>{{ (githubService$ | async)?.login || 'GitHub' }}</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/navbar/navbar.component.html` around lines 42 - 48, The anchor uses (githubService$ | async)?.html_url and (githubService$ | async)?.login which may be undefined; update the anchor to provide safe fallbacks so the CTA always has a usable href and label — e.g., when html_url is falsy use a default URL like 'https://github.com' and when login is falsy use a default label like 'GitHub' or 'View on GitHub'; locate the template fragment that binds href and the span text (the anchor element using githubService$ and properties html_url and login) and replace the direct optional bindings with expressions that fall back to these defaults.
🧹 Nitpick comments (25)
src/styles.sass (1)
7-7: Duplicate font import.This Google Fonts URL is also loaded in
src/index.html(lines 18-22). Loading the same fonts twice creates redundant network requests. Consider removing one of the imports.♻️ Proposed fix
Remove the import from SASS since
index.htmlalready loads it:-@import url('https://fonts.googleapis.com/css2?family=Inter:wght@400;500;600;700;800&family=Inter+Tight:wght@600;700;800&family=JetBrains+Mono:wght@400;500;700&family=Space+Grotesk:wght@500;700&display=swap')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/styles.sass` at line 7, The fonts are being imported twice; remove the redundant `@import` line from src/styles.sass (the line starting with "@import url('https://fonts.googleapis.com/...')") so the Google Fonts are only loaded via src/index.html; simply delete that import in the styles.sass file to prevent duplicate network requests and keep font loading centralized in index.html.src/app/experience/experience.component.ts (1)
2-2: Remove commented-out import.This commented Firebase import appears to be dead code. If it's no longer needed, remove it to keep the codebase clean.
🧹 Proposed cleanup
import { Component } from '@angular/core'; -// import { getStorage, ref } from "@angular/fire";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/experience/experience.component.ts` at line 2, Remove the commented-out Firebase import line (the "// import { getStorage, ref } from \"@angular/fire\";") from experience.component.ts; verify there are no remaining references to getStorage or ref in the ExperienceComponent class or template and, if there are none, delete the commented import to clean up dead code.src/template.css (1)
22-26: Consider removing quotes around single-word font names.Per Stylelint's
font-family-name-quotesrule, quotes aroundInterare unnecessary since it doesn't contain spaces or special characters. This also applies to line 35.♻️ Proposed fix
body { - font-family: 'Inter', 'Helvetica Neue', Helvetica, Arial, sans-serif; + font-family: Inter, 'Helvetica Neue', Helvetica, Arial, sans-serif; background-color: var(--console-bg); color: var(--console-text); }And for lines 34-36:
font-family: - 'Inter Tight', 'Inter', 'Helvetica Neue', Helvetica, Arial, sans-serif; + 'Inter Tight', Inter, 'Helvetica Neue', Helvetica, Arial, sans-serif;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/template.css` around lines 22 - 26, Remove unnecessary quotes around the single-word font name in the CSS font-family declarations: update the body selector's font-family and the other font-family instance referenced (around line 35) to use Inter without quotes (keep quotes on "Helvetica Neue" because it contains a space); target the body { font-family: ... } rule and the other font-family rule to make this change so it satisfies the stylelint font-family-name-quotes rule.scripts/format-check-staged.mjs (1)
6-10: Consider wrapping in try-catch for better error messages.If
gitis not found or fails unexpectedly, the raw error might be confusing. A try-catch with a clearer message could improve DX, though the current approach works for a pre-commit hook.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/format-check-staged.mjs` around lines 6 - 10, The execFileSync call that populates stagedOutput can throw confusing raw errors if git is missing or fails; wrap the execFileSync invocation used to set stagedOutput in a try-catch, catch any error, log or throw a clearer message that includes context (e.g., "Failed to retrieve staged files via git") and the original error details, and then exit or rethrow so the pre-commit hook fails visibly; target the stagedOutput assignment and the execFileSync call to implement this change.src/app/home/info/console-sidebar-panel.component.sass (1)
95-98: Respect reduced-motion preferences for the cursor pulse animation.Please gate the infinite pulse under
prefers-reduced-motionto avoid unnecessary motion for sensitive users.Suggested CSS patch
.console-sidebar__cursor margin-top: 0.5rem animation: pulse 1.4s ease-in-out infinite `@keyframes` pulse 0%, 100% opacity: 0.3 50% opacity: 1 + +@media (prefers-reduced-motion: reduce) + .console-sidebar__cursor + animation: noneAlso applies to: 132-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/info/console-sidebar-panel.component.sass` around lines 95 - 98, The pulse animation on .console-sidebar__cursor is currently always infinite; wrap or duplicate the rule to respect the user's reduced-motion setting by adding a prefers-reduced-motion media query that disables or reduces the animation (e.g., remove animation or set animation: none) for users who prefer reduced motion; apply the same change to the other pulse rule noted later in the file (the second .console-sidebar__cursor/pulse declaration) so both instances honor prefers-reduced-motion.src/app/ball/ball.component.html (1)
9-9: Add inline fallback text inside the canvas element.Providing fallback text in
<canvas>improves behavior for unsupported/non-visual contexts.♻️ Suggested change
- <canvas `#canvas`></canvas> + <canvas `#canvas`>{{ 'webgl_ball_unavailable' | translate }}</canvas>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/ball/ball.component.html` at line 9, Add accessible fallback text inside the <canvas> element (the element with template reference variable `#canvas`) so non-supporting or non-visual contexts get a meaningful message; update the canvas markup to include a short descriptive fallback string (e.g., "Canvas not supported" or a brief instruction) between the opening and closing <canvas> tags.src/app/app.component.html (1)
3-3: Avoid hard-coding the footer offset in content padding.Line 3 uses a fixed
pb-10; this can desync if footer height changes and clip bottom content. Consider tying this to a shared CSS variable.♻️ Suggested change
- <div class="flex-grow pb-10"> + <div class="flex-grow pb-[var(--console-footer-height,2.5rem)]">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/app.component.html` at line 3, The content wrapper currently hardcodes bottom padding via the div with class "flex-grow pb-10"; change this to use a shared CSS variable like --footer-height instead of the fixed utility class so footer size changes won't clip content. Remove the "pb-10" utility from the div (the element with class "flex-grow pb-10") and apply padding-bottom: var(--footer-height, <fallback>) via a global or component stylesheet, and ensure the footer component sets/updates --footer-height (e.g., on mount/resize) so the variable reflects the actual footer height..github/workflows/firebase-hosting-merge.yml (1)
18-18: Replace legacy npm install flag in CI.Line 18 uses
npm ci --also=dev, a flag that is no longer supported in modern npm versions. Use plainnpm ciinstead, which includes dev dependencies by default and improves compatibility across npm versions.♻️ Suggested change
- - run: npm ci --also=dev + - run: npm ci🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/firebase-hosting-merge.yml at line 18, The workflow uses the deprecated npm flag "npm ci --also=dev"; update the CI step so the command string "npm ci --also=dev" is replaced with plain "npm ci" to ensure compatibility with modern npm versions and to still install devDependencies by default.src/app/footer/footer.component.spec.ts (1)
41-46: Avoid locale-coupled link assertion by visible text.This assertion is brittle with translation changes; prefer checking for an anchor with router binding/target instead of exact
"Curriculum"text.🔧 Suggested patch
- const curriculumLink = fixture.debugElement - .queryAll(By.css('.console-footer__links a')) - .find((link) => link.nativeElement.textContent.trim() === 'Curriculum'); + const curriculumLink = fixture.debugElement.query( + By.css('.console-footer__links a[routerLink], .console-footer__links a[href]') + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/footer/footer.component.spec.ts` around lines 41 - 46, The test currently asserts an anchor by its visible text "Curriculum", which is locale-coupled; instead locate the anchor by its router binding or router directive. In the 'should render a curriculum router link' test, replace the text-based find on fixture.debugElement.queryAll(By.css('.console-footer__links a')) with a lookup that checks each DebugElement for a router link (e.g. presence of attributes['routerLink'] or the RouterLinkWithHref directive) and assert that such an element exists (keep the curriculumLink variable and expect(curriculumLink).toBeTruthy()). This removes dependency on translated text while still validating the router link rendering.karma.conf.js (1)
12-24: Verify coverage report output configuration.The migration from
karma-coverage-istanbul-reportertokarma-coverageremoved the explicitcoverageIstanbulReporterconfiguration (output directory, report formats). Withkarma-coverage, you may want to add acoverageReporterblock to specify output directory and report types, especially if CI pipelines depend on specific report locations.🔧 Optional: Add explicit coverage configuration
reporters: ['progress', 'kjhtml', 'coverage'], + coverageReporter: { + dir: require('path').join(__dirname, './coverage'), + subdir: '.', + reporters: [ + { type: 'html' }, + { type: 'lcov' }, + { type: 'text-summary' }, + ], + }, port: 9876,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@karma.conf.js` around lines 12 - 24, The test config now uses karma-coverage but lacks an explicit coverage output config; add a coverageReporter block to the Karma config to set the output directory and report formats so CI and tooling still find reports (e.g., add a coverageReporter object near reporters that specifies dir, reporters array like 'lcov' and 'text-summary', and subdir handling). Update the configuration in the same file where reporters is defined (reference: reporters, customLaunchers/ChromeHeadlessNoSandbox) and ensure formats and output paths match the CI expectations.src/app/experience/viewer/viewer.component.spec.ts (1)
45-53: Consider using a named constant for the zoom lower bound.The assertion
toBeGreaterThan(0.39)uses a magic number. If the component's minimum zoom constant changes, this test may silently pass or fail unexpectedly.💡 Suggestion
Consider importing or referencing the component's minimum zoom constant directly, or add a comment explaining the expected minimum:
it('should zoom in and out respecting lower bound', () => { const initial = component.zoom; component.zoomIn(); expect(component.zoom).toBeCloseTo(initial + 0.1, 5); for (let i = 0; i < 10; i++) { component.zoomOut(); } - expect(component.zoom).toBeGreaterThan(0.39); + // Minimum zoom is 0.4 (10 zoom-outs from ~1.1 should hit the floor) + expect(component.zoom).toBeGreaterThanOrEqual(0.4); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/experience/viewer/viewer.component.spec.ts` around lines 45 - 53, Test uses a magic number 0.39 for the minimum zoom; replace it by referencing the component's min-zoom constant (e.g., ViewerComponent.MIN_ZOOM or component.MIN_ZOOM) or import the exported MIN_ZOOM so the test follows the actual implementation. Update the assertion to compare against that constant (use toBeGreaterThan or toBeGreaterThanOrEqual with a small epsilon if needed) and remove the hard-coded 0.39 literal.src/app/home/info/console-left-rail.component.html (1)
9-36: Consider extracting the repeated link template pattern.The same conditional link-rendering logic (external
<a href>vs internal<a routerLink>) is duplicated forprimaryItems()andutilityItems(). While functional, this increases maintenance burden.Consider extracting this into an
ng-templatewith context or a small presentational component to reduce duplication.♻️ Example using ng-template
<ng-template `#linkTemplate` let-item let-minor="minor"> `@if` (item.href) { <a [href]="item.href" target="_blank" rel="noopener noreferrer" [class]="'console-rail__link' + (minor ? ' console-rail__link--minor' : '')" [class.console-rail__link--active]="item.active" > <span class="material-symbols-outlined" aria-hidden="true">{{ item.icon }}</span> <span>{{ item.label }}</span> </a> } `@else` { <a [routerLink]="item.routerLink" [fragment]="item.fragment" [class]="'console-rail__link' + (minor ? ' console-rail__link--minor' : '')" [class.console-rail__link--active]="item.active" > <span class="material-symbols-outlined" aria-hidden="true">{{ item.icon }}</span> <span>{{ item.label }}</span> </a> } </ng-template>Also applies to: 45-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/info/console-left-rail.component.html` around lines 9 - 36, The repeated conditional link rendering for primaryItems() and utilityItems() should be extracted to avoid duplication: create an ng-template (e.g., linkTemplate) or a small presentational component that accepts an item (and an optional minor flag), move the shared markup that checks item.href vs routerLink/fragment and renders the <a> with classes console-rail__link and console-rail__link--active, and then replace the two `@for` loops to invoke the template/component with each item; ensure the template uses item.href to decide external vs routerLink rendering and preserves item.icon and item.label.src/app/home/info/info.component.spec.ts (1)
45-47: Consider adding more meaningful tests.The test suite only verifies component creation. Given the new console-themed UI and service integrations, consider adding tests for the component's data bindings, signal outputs, and conditional rendering logic.
Would you like me to help generate additional test cases for
InfoComponent?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/info/info.component.spec.ts` around lines 45 - 47, Test suite only checks InfoComponent creation; add focused tests to cover bindings, outputs and conditional rendering. In InfoComponent spec, add tests that (1) verify template displays input-bound data (e.g., component.title or component.consoleTheme) by setting those properties and triggering change detection and querying the DOM; (2) assert signal/observable outputs or emitted events from methods like onToggleConsole or any EventEmitter by subscribing/spying and invoking the method; (3) cover conditional rendering branches (e.g., show/hide elements when component.isLoading or hasData is true/false) by toggling those flags and checking DOM presence; and (4) mock and inject related services (e.g., InfoService) to return controlled data and assert the component reacts correctly. Use existing symbols InfoComponent, any output EventEmitter or signal names, onToggleConsole (or equivalent), isLoading/hasData, and InfoService to locate targets in the spec.src/app/home/info/console-sidebar-panel.component.html (1)
27-37: Consider using a more stable track expression.The track expression
entry.time + entry.messagerelies on string concatenation. If logs have a unique identifier (e.g.,idorindex), prefer that to avoid unnecessary DOM churn when eithertimeormessagechanges independently.💡 Suggested improvement if a unique ID exists
- `@for` (entry of logs(); track entry.time + entry.message) { + `@for` (entry of logs(); track entry.id) {Alternatively, if logs are append-only and order is stable,
$indexcould work:- `@for` (entry of logs(); track entry.time + entry.message) { + `@for` (entry of logs(); track $index) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/info/console-sidebar-panel.component.html` around lines 27 - 37, The ngFor track expression currently uses string concatenation (entry.time + entry.message) which is unstable; update the trackBy expression in the template to use a stable unique key such as entry.id (if each log object has an id) or, if logs are strictly append-only and order never reorders, use the $index as the track identifier; locate the ngFor using logs() and the entry variable in the console-sidebar-panel template and replace the track expression accordingly so DOM elements aren't recreated when time or message change independently.src/app/ball/ball.component.sass (1)
25-33: Consider using CSS variables for theme colors.The hardcoded hex values
#67e8f9and#a5f3fcappear to be console theme colors. If the project has CSS custom properties for the color palette, consider using them for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/ball/ball.component.sass` around lines 25 - 33, Replace the hardcoded hex color values in the anchor rules with the project's CSS custom properties: update the color property on the "a" selector and the color used in the nested &:hover, &:focus, &:focus-visible to reference the theme variables (e.g., use the project's --* color tokens with fallbacks) so the link color follows the centralized palette; target the "a" block in src/app/ball/ball.component.sass and adjust the color declarations accordingly.src/app/app.component.spec.ts (1)
4-4:RouterTestingModuleis deprecated as of Angular 17.Consider migrating to
provideRouter()withRouterModuleorRouterTestingHarnessfor modern router testing. An official migration schematic is available viang generate@angular/core:router-testing-module-migration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/app.component.spec.ts` at line 4, The test imports RouterTestingModule which is deprecated; replace its usage by configuring the test with provideRouter() (or using RouterTestingHarness) instead: remove RouterTestingModule import and any uses of RouterTestingModule in TestBed.configureTestingModule, add a provider created via provideRouter([...]) with your app routes or a minimal stub route in the spec, and adapt any router-related injections to use Router from TestBed.inject or use RouterTestingHarness utilities (e.g., replace RouterTestingModule.withRoutes(...) with provideRouter([...]) or use RouterTestingHarness.navigateByUrl in tests). Update imports to include provideRouter or RouterTestingHarness and adjust setup in the spec file (app.component.spec.ts) to initialize the router via the new provider so tests run without the deprecated RouterTestingModule.src/app/services/github.service.spec.ts (2)
44-60: Conditional assertion in subscribe may cause test flakiness.The
if (user.login === 'foo')check suggests the service emits multiple values. If the default user emits first, thedone()callback won't be called untilfoois received. However, if the service behavior changes or the order differs, this test could hang or pass incorrectly. Consider using RxJS testing utilities or explicitly handling the emission sequence.♻️ Alternative approach using skip or take
it('should fetch given user', (done) => { + // Skip the initial default user emission service.subscribe((user) => { - if (user.login === 'foo') { - expect(user.name).toBe('Foo Bar'); - done(); - } + expect(user.login).toBe('foo'); + expect(user.name).toBe('Foo Bar'); + done(); }); + // Flush default request first if service auto-fetches service.checkUser('foo'); const req = http.expectOne('https://api.github.com/users/foo');Or use
pipe(skip(1), take(1))to explicitly skip the default emission if the service auto-fetches on initialization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/services/github.service.spec.ts` around lines 44 - 60, The test is flaky because service.subscribe(...) may receive a default emission before the 'foo' response; change the subscription to explicitly consume the expected emission sequence (e.g., subscribe to service.pipe(skip(1), take(1)) or use firstValueFrom(service.pipe(skip(1)))) so you no longer rely on an if (user.login === 'foo') guard; then assert user.name === 'Foo Bar', call done(), and keep the existing service.checkUser('foo') and http.expectOne('https://api.github.com/users/foo')/req.flush(...) flow to trigger the emission.
17-27: AddafterEachto verify no outstanding HTTP requests.For consistency with other service specs in this PR, add
http.verify()inafterEach.♻️ Proposed fix
service = TestBed.inject(GithubService); http = TestBed.inject(HttpTestingController); }); + + afterEach(() => { + http.verify(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/services/github.service.spec.ts` around lines 17 - 27, Add an afterEach block that calls http.verify() to ensure no outstanding HTTP requests after each test; locate the test setup that uses TestBed.configureTestingModule and the injected GithubService and HttpTestingController (variables service and http) and add an afterEach(() => http.verify()) to match other service specs.src/app/services/country.service.spec.ts (1)
17-27: AddafterEachto verify no outstanding HTTP requests.The other service specs in this PR (e.g.,
personal-info.service.spec.ts) includehttpMock.verify()inafterEachto catch unexpected requests. Adding this here would maintain consistency and improve test reliability.♻️ Proposed fix
beforeEach(() => { TestBed.configureTestingModule({ imports: [], providers: [ provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting(), ], }); service = TestBed.inject(CountryService); http = TestBed.inject(HttpTestingController); }); + + afterEach(() => { + http.verify(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/services/country.service.spec.ts` around lines 17 - 27, Add an afterEach block to verify there are no outstanding HTTP requests by calling http.verify() (matching the HttpTestingController instance named "http" used in beforeEach); place this afterEach alongside the existing beforeEach in the CountryService spec so tests fail when unexpected or unflushed HTTP requests remain, ensuring consistency with other specs like personal-info.service.spec.ts.src/app/experience/viewer/viewer.component.sass (1)
83-87: Add explicit keyboard focus treatment for action controls.Line 83 adds hover states, but there is no matching
:focus-visiblestyle for keyboard navigation.♿ Suggested patch
.experience-viewer__action:hover, .experience-viewer__button:hover border-color: rgba(173, 198, 255, 0.28) background: rgba(26, 36, 61, 0.95) + +.experience-viewer__action:focus-visible, +.experience-viewer__button:focus-visible + outline: 2px solid var(--console-primary) + outline-offset: 2px + border-color: rgba(173, 198, 255, 0.4)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/experience/viewer/viewer.component.sass` around lines 83 - 87, The hover-only styles for .experience-viewer__action and .experience-viewer__button lack keyboard focus styling—add matching :focus-visible rules so keyboard users get the same visual affordance (border-color and background) and ensure an accessible outline or box-shadow is present (e.g., a clear 2px outline or focus ring) to mirror the hover state; update the .experience-viewer__action:focus-visible and .experience-viewer__button:focus-visible selectors to apply the same border-color/background and an explicit focus indicator, and ensure :focus is not used alone if you want to preserve mouse focus behavior.src/app/common/console/console.component.spec.ts (1)
13-24: Tighten theCountryServicetest double to match method contract.Current stub puts
subscribeon the service object itself; it’s cleaner to return an observable fromcheckIP.🧪 Suggested patch
- let countryServiceStub: { - checkIP: jasmine.Spy; - subscribe: typeof Observable.prototype.subscribe; - }; + let countryServiceStub: { + checkIP: jasmine.Spy; + }; @@ - checkIPSpy = jasmine.createSpy('checkIP'); const countryResult$ = of({ ip: '', country: 'US' }); + checkIPSpy = jasmine.createSpy('checkIP').and.returnValue(countryResult$); countryServiceStub = { checkIP: checkIPSpy, - subscribe: countryResult$.subscribe.bind(countryResult$), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/common/console/console.component.spec.ts` around lines 13 - 24, The test double for CountryService should expose checkIP as a method that returns an Observable instead of putting subscribe on the stub object; update the stub so checkIP (the jasmine spy checkIPSpy used in the beforeEach) is configured to return the observable countryResult$ (e.g., checkIPSpy.and.returnValue(countryResult$)) and remove the subscribe property from countryServiceStub so the test uses countryServiceStub.checkIP() which returns an Observable consistent with the real CountryService contract.src/app/footer/footer.component.html (1)
19-22: Move repository URL composition out of the template.Line 19-22 mixes async extraction and string concatenation inline, which is harder to maintain and test.
♻️ Suggested patch
- <a - [href]=" - ((githubService$ | async)?.html_url || 'https://github.com/FarDust') + - '/fardust-web' - " + <a + [href]="sourceRepositoryUrl$ | async" target="_blank" rel="noopener noreferrer" >// footer.component.ts (supporting change outside selected lines) sourceRepositoryUrl$ = this.githubService$.pipe( map((profile) => `${(profile?.html_url ?? 'https://github.com/FarDust').replace(/\/+$/, '')}/fardust-web`) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/footer/footer.component.html` around lines 19 - 22, Move the inline URL construction out of the template by adding a new observable property (e.g. sourceRepositoryUrl$) in FooterComponent that maps githubService$ to a single sanitized repository URL (use profile?.html_url with a fallback, strip trailing slashes, then append "/fardust-web"), and update the template to bind [href] to sourceRepositoryUrl$ | async instead of the current ((githubService$ | async)?.html_url || ...) + '/fardust-web'; this centralizes logic for easier testing and maintenance.src/app/home/info/console-left-rail.component.sass (1)
73-82: Mirror hover styling with:focus-visibleon rail links.Lines 73-82 style hover/active, but keyboard focus feedback is not explicitly defined.
♿ Suggested patch
.console-rail__link:hover border-color: rgba(173, 198, 255, 0.18) background: rgba(19, 27, 46, 0.72) color: `#dae2fd` + +.console-rail__link:focus-visible + outline: 2px solid var(--console-primary) + outline-offset: 2px + border-color: rgba(173, 198, 255, 0.24) + background: rgba(19, 27, 46, 0.86) + color: `#dae2fd`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/info/console-left-rail.component.sass` around lines 73 - 82, Add keyboard focus styling by adding :focus-visible rules that mirror the existing hover/active states: create .console-rail__link:focus-visible with the same border-color, background and color as .console-rail__link:hover, and .console-rail__link--active:focus-visible matching .console-rail__link--active; ensure these selectors provide identical visual feedback for keyboard users (and remove any default outline if you intentionally replicate the hover appearance).src/app/experience/viewer/viewer.component.html (1)
19-26: Finish localizing the new viewer chrome.This template mixes translated actions with hardcoded headers and status labels (
Artifact Browser,Pdf Loaded,PROFILE_DETAILS,DOCUMENT_STATE, etc.), so locale switching will still leave most of the viewer in English.Also applies to: 49-55, 107-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/experience/viewer/viewer.component.html` around lines 19 - 26, Replace hardcoded UI strings in the viewer template with translated keys and the translate pipe: change the kicker text (element with class experience-viewer__kicker currently "Artifact Browser"), the description paragraph (experience-viewer__copy), and the other hardcoded status/label strings (e.g., "Pdf Loaded", "PROFILE_DETAILS", "DOCUMENT_STATE" mentioned elsewhere) to use i18n keys via {{ 'your_key' | translate }} (or the project's translation helper) so all viewer chrome is localized; update corresponding translation keys in the locale resource files and ensure the existing {{ 'cv_title' | translate }} pattern is applied consistently across the blocks referenced (lines ~49-55 and ~107-152).src/app/home/info/console-project-registry.component.html (1)
51-54: Make the GitHub tile actionable or clearly non-interactive.
MORE_REFERENCES_ON_GITHUBreads like the next selectable card in the grid, but as a plaindivit is neither clickable nor keyboard-focusable. Either wire it to the intended GitHub destination or relabel/restyle it as a non-action placeholder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/info/console-project-registry.component.html` around lines 51 - 54, The placeholder tile with class "console-registry__placeholder" and text "MORE_REFERENCES_ON_GITHUB" is currently a non-interactive div but looks like a selectable card; either make it a real link or make clear it's a non-action item: to make it actionable, replace the div with an anchor or button pointing to the intended GitHub URL, keep the same CSS class, add keyboard and screen-reader affordances (e.g., tabindex, aria-label, target="_blank" and rel="noopener noreferrer" if opening a new tab); alternatively, if it should be non-interactive, update the markup/text to a non-action label (e.g., change copy to indicate "No more references" or add aria-hidden and a visual style that indicates disabled/non-clickable) and ensure no tabindex is present so it is not keyboard-focusable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5ebcbfed-52de-46ec-85e9-d2207e779c03
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (93)
.coderabbit.yaml.eslintrc.json.github/workflows/firebase-hosting-cleanup.yml.github/workflows/firebase-hosting-merge.yml.github/workflows/firebase-hosting-pull-request.yml.github/workflows/node-ci.yml.prettierrcAGENTS.mdREADME.mdangular.jsonkarma.conf.jspackage.jsonscripts/format-check-staged.mjssrc/app/app-routing.module.tssrc/app/app.component.htmlsrc/app/app.component.spec.tssrc/app/app.component.tssrc/app/app.module.tssrc/app/ball/ball-routing.module.tssrc/app/ball/ball.component.htmlsrc/app/ball/ball.component.sasssrc/app/ball/ball.component.spec.tssrc/app/ball/ball.component.tssrc/app/ball/ball.module.tssrc/app/common/console/console.component.htmlsrc/app/common/console/console.component.spec.tssrc/app/common/console/console.component.tssrc/app/common/console/console.module.tssrc/app/experience/experience.component.tssrc/app/experience/experience.module.tssrc/app/experience/viewer/viewer.component.htmlsrc/app/experience/viewer/viewer.component.sasssrc/app/experience/viewer/viewer.component.spec.tssrc/app/experience/viewer/viewer.component.tssrc/app/footer/footer.component.htmlsrc/app/footer/footer.component.sasssrc/app/footer/footer.component.spec.tssrc/app/footer/footer.component.tssrc/app/home/home.component.tssrc/app/home/home.module.tssrc/app/home/info/console-hero-panel.component.htmlsrc/app/home/info/console-hero-panel.component.sasssrc/app/home/info/console-hero-panel.component.tssrc/app/home/info/console-left-rail.component.htmlsrc/app/home/info/console-left-rail.component.sasssrc/app/home/info/console-left-rail.component.tssrc/app/home/info/console-pipeline-panel.component.htmlsrc/app/home/info/console-pipeline-panel.component.sasssrc/app/home/info/console-pipeline-panel.component.tssrc/app/home/info/console-profile-badge.component.htmlsrc/app/home/info/console-profile-badge.component.sasssrc/app/home/info/console-profile-badge.component.spec.tssrc/app/home/info/console-profile-badge.component.tssrc/app/home/info/console-project-registry.component.htmlsrc/app/home/info/console-project-registry.component.sasssrc/app/home/info/console-project-registry.component.tssrc/app/home/info/console-shell.data.tssrc/app/home/info/console-sidebar-panel.component.htmlsrc/app/home/info/console-sidebar-panel.component.sasssrc/app/home/info/console-sidebar-panel.component.tssrc/app/home/info/console.models.tssrc/app/home/info/info.component.htmlsrc/app/home/info/info.component.sasssrc/app/home/info/info.component.spec.tssrc/app/home/info/info.component.tssrc/app/home/personal-info/personal-info.component.htmlsrc/app/home/personal-info/personal-info.component.sasssrc/app/home/personal-info/personal-info.component.spec.tssrc/app/home/personal-info/personal-info.component.tssrc/app/navbar/navbar.component.htmlsrc/app/navbar/navbar.component.sasssrc/app/navbar/navbar.component.spec.tssrc/app/navbar/navbar.component.tssrc/app/page-not-found/page-not-found.component.htmlsrc/app/page-not-found/page-not-found.component.spec.tssrc/app/page-not-found/page-not-found.component.tssrc/app/services/country.service.spec.tssrc/app/services/github.service.spec.tssrc/app/services/personal-info.service.spec.tssrc/app/services/personal-info.service.tssrc/assets/i18n/en.jsonsrc/assets/i18n/es.jsonsrc/environments/environment.prod.tssrc/environments/environment.tssrc/index.htmlsrc/main.tssrc/polyfills.tssrc/styles.sasssrc/template.csssrc/test.tssrc/theme/colors.sasstailwind.config.jstsconfig.json
Comment on lines
+20
to
+28
| "@angular/animations": "~21.2.5", | ||
| "@angular/common": "~21.2.5", | ||
| "@angular/compiler": "~21.2.5", | ||
| "@angular/core": "~21.2.5", | ||
| "@angular/forms": "~21.2.5", | ||
| "@angular/platform-browser": "~21.2.5", | ||
| "@angular/platform-browser-dynamic": "~21.2.5", | ||
| "@angular/router": "~21.2.5", | ||
| "@angular/service-worker": "~21.2.5", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Angular 21 release date compatibility
💡 Result:
Angular 21 was released on November 19, 2025. It is actively supported until May 19, 2026 (6 months active), with LTS until May 19, 2027. Compatibility requirements for Angular 21.0.x: - Node.js: ^20.19.0 || ^22.12.0 || ^24.0.0 - TypeScript: >=5.9.0 <6.0.0 - RxJS: ^6.5.3 || ^7.4.0 - Browser support: Browsers in the "widely available" Baseline as of 2025-10-20 (Chrome, Edge, Firefox, Safari versions covering ~95% of users, released <30 months prior). Update from Angular 20 to 21 using the official ng update guide at https://v21.angular.dev/update-guide. Angular follows semantic versioning with major releases every 6 months.
Citations:
- 1: https://versionlog.com/angular/21.0/
- 2: https://blog.angular.dev/announcing-angular-v21-57946c34f14b
- 3: https://v21.angular.dev/reference/versions
- 4: https://angular.dev/reference/versions
- 5: https://angular.dev/reference/releases
- 6: https://en.wikipedia.org/wiki/Angular_(web_framework)
🏁 Script executed:
cat -n package.json | head -40Repository: FarDust/fardust-web
Length of output: 1735
🌐 Web query:
Angular 21.2.x release version
💡 Result:
The latest version in the Angular 21.2.x series is 21.2.5.
Citations:
- 1: https://registry.npmjs.org/%40angular%2Fcore
- 2: https://www.npmjs.com/package/@angular/core?activeTab=versions
- 3: https://github.com/angular/angular/releases
- 4: https://www.npmjs.com/package/@angular/core
🌐 Web query:
@ngx-translate/core 14.0.0 Angular 21 compatibility
💡 Result:
No, @ngx-translate/core version 14.0.0 is not compatible with Angular 21. It was released on November 8, 2021, and is built for Angular 13 (Ivy only), requiring Angular >=13 and <16. The official compatibility table confirms it supports Angular 13-15. Angular 21 requires @ngx-translate/core 17.x. Users report it works with Angular 14, but not guaranteed for higher versions due to breaking changes in later ngx-translate releases. Upgrade to 17.x for Angular 21 compatibility.
Citations:
- 1: https://ngx-translate.org/getting-started/angular-compatibility/
- 2: https://ngx-translate.org/getting-started/angular-compatibility
- 3: https://github.com/ngx-translate/core/releases
- 4: Angular 14 support? ngx-translate/core#1378
- 5: https://www.npmjs.com/package/@ngx-translate/core/v/14.0.0
🌐 Web query:
zone.js 0.15.1 Angular 21 compatibility
💡 Result:
zone.js 0.15.1 is compatible with Angular 21 when using zone-based change detection. Angular 21 makes zoneless change detection the default for new projects (no zone.js required), but fully supports zone.js for existing apps or opt-in use. zone.js became an optional peer dependency in Angular around v20+, and 0.15.x (released 2024-2025) aligns with Angular 19-21 era updates. No specific incompatibilities found for 0.15.1 with Angular 21; later zone.js versions like 0.16 exist but are not required. For zoneless apps, remove zone.js entirely to reduce bundle size (~33KB savings) and improve performance.
Citations:
- 1: https://www.pkgpulse.com/blog/angular-21-zoneless-zone-js-performance-2026
- 2: https://angular.dev/reference/versions
- 3: https://github.com/angular/angular/blob/main/packages/zone.js/CHANGELOG.md
- 4: https://www.npmjs.com/package/zone.js?activeTab=readme
- 5: https://blog.angular.dev/announcing-angular-v21-57946c34f14b
- 6: https://angular.dev/guide/zoneless
- 7: [main] build: update zone.js peer dependency for core package to 0.15.0 angular/angular#57431
- 8: fix(@angular/core): mark
zone.jsas an optional peer dependency angular/angular#61616
@ngx-translate/core 14.0.0 is incompatible with Angular 21—upgrade to 17.x required.
Angular 21.2.5 is officially released and in active support. However, @ngx-translate/core 14.0.0 (currently in package.json at line 34) was built for Angular 13–15 and is incompatible with Angular 21. The compatible version for Angular 21 is @ngx-translate/core 17.x. zone.js 0.15.1 is compatible with Angular 21 and requires no changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 20 - 28, The package.json currently pins
"@ngx-translate/core" to an incompatible 14.0.0 for Angular 21; update the
dependency to a 17.x compatible release (e.g., "@ngx-translate/core": "^17.0.0")
in package.json, run npm/yarn install to update the lockfile, and run the
test/build steps to confirm no peer dependency issues; also verify any
imports/usages around TranslateModule or TranslateService still work and
resolve/upgrade related `@ngx-translate` packages if present.
Comment on lines
+5
to
+11
| @Component({ | ||
| selector: 'app-console-pipeline-panel', | ||
| imports: [CommonModule], | ||
| templateUrl: './console-pipeline-panel.component.html', | ||
| styleUrls: ['./console-pipeline-panel.component.sass'], | ||
| changeDetection: ChangeDetectionStrategy.OnPush, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if other console components in this directory also use imports without standalone
rg -n "imports:" src/app/home/info/*.component.ts -A2 -B5 | head -60Repository: FarDust/fardust-web
Length of output: 3731
🏁 Script executed:
cat -n src/app/home/info/console-pipeline-panel.component.tsRepository: FarDust/fardust-web
Length of output: 1089
🏁 Script executed:
cat -n src/app/home/info/console-sidebar-panel.component.ts | head -30Repository: FarDust/fardust-web
Length of output: 1410
🏁 Script executed:
cat package.json | grep -A 5 "@angular"Repository: FarDust/fardust-web
Length of output: 1257
🏁 Script executed:
# Check if all console components follow the same pattern
rg "@Component" src/app/home/info/*.component.ts -A 10 | grep -E "(standalone|imports)" | head -20Repository: FarDust/fardust-web
Length of output: 561
🌐 Web query:
Angular 21.2.5 imports property standalone true requirement
💡 Result:
In Angular 21.2.5, setting the standalone: true property in a component's @Component decorator is required to define a standalone component. Standalone components declare their dependencies directly in the imports array instead of relying on NgModules. This has been the standard requirement since standalone components were introduced in Angular 14 (stable in 15+), with no changes in 21.2.5. Angular 21 emphasizes standalone as the default/recommended approach (new apps generated without zone.js, Vitest default), but standalone: true remains explicitly required for components, directives, and pipes to be standalone. Components without it default to non-standalone (module-based). Example: @Component({ standalone: true, imports: [CommonModule, OtherComponent], selector: 'app-example', template:
Example
}) export class ExampleComponent {} Official docs confirm: standalone components must set standalone: true and populate imports for template dependencies. Release notes for v21.2.x (including patches up to 21.2.4 shown) have no breaking changes or new requirements for this.
Citations:
- 1: https://github.com/angular/angular/releases/tag/v21.2.0
- 2: https://v17.angular.io/guide/standalone-components
- 3: https://v18.angular.dev/guide/components/importing
- 4: https://angular.dev/guide/components
- 5: https://angular.dev/reference/migrations/standalone
- 6: https://github.com/angular/angular/blob/main/CHANGELOG.md
- 7: https://www.angular.dev
🏁 Script executed:
# Check if there are any TypeScript errors or compilation issues
# Look for any configuration that might handle this
cat tsconfig.json | head -30Repository: FarDust/fardust-web
Length of output: 612
🏁 Script executed:
# Check if there's a build or compilation configuration
ls -la | grep -E "(angular|ng|build|tsconfig)" | head -20Repository: FarDust/fardust-web
Length of output: 461
Add standalone: true to component decorator.
The component uses the imports array (line 7) which requires standalone: true to be declared in the decorator according to Angular 21.2.5 specification. Without it, the component will not function as a standalone component and may cause compilation issues.
Proposed fix
`@Component`({
selector: 'app-console-pipeline-panel',
+ standalone: true,
imports: [CommonModule],
templateUrl: './console-pipeline-panel.component.html',
styleUrls: ['./console-pipeline-panel.component.sass'],
changeDetection: ChangeDetectionStrategy.OnPush,
})📝 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.
Suggested change
| @Component({ | |
| selector: 'app-console-pipeline-panel', | |
| imports: [CommonModule], | |
| templateUrl: './console-pipeline-panel.component.html', | |
| styleUrls: ['./console-pipeline-panel.component.sass'], | |
| changeDetection: ChangeDetectionStrategy.OnPush, | |
| }) | |
| `@Component`({ | |
| selector: 'app-console-pipeline-panel', | |
| standalone: true, | |
| imports: [CommonModule], | |
| templateUrl: './console-pipeline-panel.component.html', | |
| styleUrls: ['./console-pipeline-panel.component.sass'], | |
| changeDetection: ChangeDetectionStrategy.OnPush, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/home/info/console-pipeline-panel.component.ts` around lines 5 - 11,
The `@Component` decorator for the component with selector
'app-console-pipeline-panel' declares an imports array but is missing
standalone: true; update the component decorator to include standalone: true so
the imports array is valid under Angular 21.2.5 (i.e., add standalone: true
alongside selector, imports, templateUrl, styleUrls, changeDetection in the
`@Component` metadata for this component).
Comment on lines
+81
to
+84
| private persistToken(token: string): void { | ||
| const secure = window.location.protocol === 'https:' ? '; Secure' : ''; | ||
| document.cookie = `${TOKEN_COOKIE}=${encodeURIComponent(token)}; path=/; SameSite=Strict${secure}`; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect PersonalInfo token usage and whether it is credential-like.
fd personal-info.service.ts --exec sed -n '1,220p' {}
rg -n -C3 "getPersonalInfo\\(|Authorization|Bearer|token|personal_token"Repository: FarDust/fardust-web
Length of output: 50376
🏁 Script executed:
rg -n "HttpOnly|httpOnly|secure.*cookie|session|Backend.*exchange" src/app/home/personal-info/ -A 2 -B 2Repository: FarDust/fardust-web
Length of output: 45
🏁 Script executed:
# Check if there are alternative storage mechanisms or backend session management
rg -n "localStorage|sessionStorage|IndexedDB|Bearer" src/app/home/personal-info/ -A 1Repository: FarDust/fardust-web
Length of output: 45
🏁 Script executed:
# Look for any XSS protections or sanitization
rg -n "DomSanitizer|sanitize|CSP|Content-Security-Policy" src/app/ --max-count=10Repository: FarDust/fardust-web
Length of output: 45
🏁 Script executed:
# Search for auth-related patterns more broadly
rg -n "setToken|getToken|Authorization.*Bearer|authToken|personalToken" src/ --max-count=15Repository: FarDust/fardust-web
Length of output: 272
🏁 Script executed:
# Check the overall app structure for auth mechanisms
fd -t f "(auth|token|service)" src/app/ | head -20Repository: FarDust/fardust-web
Length of output: 303
🏁 Script executed:
# Verify the component that calls persistToken
rg -n "persistToken" src/app/home/personal-info/ -B 5 -A 2Repository: FarDust/fardust-web
Length of output: 1452
Refactor token persistence to use HttpOnly session cookies instead of JS-accessible storage.
The token on line 83 is a Bearer credential that grants access to private user data. Storing it in a JS-readable cookie—even with Secure and SameSite=Strict flags—leaves it vulnerable to exfiltration via XSS attacks. These flags mitigate HTTPS downgrade and CSRF but not XSS.
Replace the current document.cookie approach with a backend-managed HttpOnly session cookie, or use a short-lived token with a backend refresh mechanism that keeps sensitive credentials server-side only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/home/personal-info/personal-info.component.ts` around lines 81 - 84,
persistToken currently writes a JS-accessible cookie (TOKEN_COOKIE) which
exposes the Bearer token to XSS; instead remove client-side cookie writes and
send the token to the backend so the server can create an HttpOnly, Secure,
SameSite session cookie via Set-Cookie. Concretely: replace the body of
persistToken(token: string) so it calls an auth/session API (e.g., POST /session
or /auth/token) passing the token in the request body or Authorization header,
await the response, and let the server set TOKEN_COOKIE as HttpOnly (and handle
refresh/expiry server-side); update any callers of persistToken to handle the
async call and error paths, and remove any other direct uses of document.cookie
for TOKEN_COOKIE.
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns the landing and curriculum areas into a single “console” shell experience while modernizing the Angular toolchain and adding i18n + a few new features (personal info + WebGL demo).
Changes:
- Upgrades Angular/TypeScript/tooling and modernizes HTTP/testing setup (
provideHttpClient, new Karma coverage plugin, Prettier). - Introduces console-styled layout components reused by landing + curriculum viewer (navbar/footer/route chrome + left rail/panels).
- Adds i18n (EN/ES), personal info feature (token via
Authorizationheader), anchor scrolling, and a/ballWebGL demo route.
Reviewed changes
Copilot reviewed 91 out of 94 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Updates module resolution for newer Angular/TS tooling. |
| tailwind.config.js | Extends Tailwind theme with console fonts + animations. |
| src/theme/colors.sass | Adjusts theme palette to the console-inspired colors. |
| src/test.ts | Updates Zone testing import + testbed init formatting. |
| src/template.css | Adds CSS variables and console route chrome styling. |
| src/styles.sass | Updates global typography/colors and console styling primitives. |
| src/polyfills.ts | Updates Zone polyfill import path for newer zone.js. |
| src/main.ts | Adjusts bootstrap configuration for updated Angular runtime settings. |
| src/index.html | Updates meta, fonts, and theme color; adds font preconnect. |
| src/environments/environment.ts | Adds personalInfoUrl configuration flag (dev). |
| src/environments/environment.prod.ts | Adds personalInfoUrl configuration flag (prod). |
| src/assets/i18n/es.json | Adds Spanish translations for new UI strings. |
| src/assets/i18n/en.json | Adds English translations for new UI strings. |
| src/app/services/personal-info.service.ts | New service for private info fetch with Bearer token auth. |
| src/app/services/personal-info.service.spec.ts | Adds unit tests for PersonalInfoService behaviors. |
| src/app/services/github.service.spec.ts | Migrates GitHub service tests to functional HTTP testing providers. |
| src/app/services/country.service.spec.ts | Migrates Country service tests to functional HTTP testing providers. |
| src/app/page-not-found/page-not-found.component.ts | Adds i18n-ready template and updated component metadata. |
| src/app/page-not-found/page-not-found.component.spec.ts | Updates tests to include TranslateModule. |
| src/app/page-not-found/page-not-found.component.html | Replaces placeholder text with translated string. |
| src/app/navbar/navbar.component.ts | Adds route-aware UI helpers and menu close behavior. |
| src/app/navbar/navbar.component.spec.ts | Updates testbed setup and adds menu behavior tests. |
| src/app/navbar/navbar.component.sass | New console navbar styling. |
| src/app/navbar/navbar.component.html | New console navbar markup + router links + mobile menu. |
| src/app/home/personal-info/personal-info.component.ts | New standalone component to manage token cookie + private fetch. |
| src/app/home/personal-info/personal-info.component.spec.ts | Adds unit tests for cookie/token and error handling flows. |
| src/app/home/personal-info/personal-info.component.sass | Styles for public/private personal-info panel. |
| src/app/home/personal-info/personal-info.component.html | Personal info UI with token input, actions, and async rendering. |
| src/app/home/info/info.component.ts | Replaces legacy landing layout with console shell data + panels. |
| src/app/home/info/info.component.spec.ts | Updates testbed providers/imports for new console UI. |
| src/app/home/info/info.component.sass | Adds responsive console grid layout styling. |
| src/app/home/info/info.component.html | New console layout composition (rail/hero/pipeline/registry/sidebar). |
| src/app/home/info/console.models.ts | Defines typed models for console UI data. |
| src/app/home/info/console-sidebar-panel.component.ts | Sidebar panel component including personal info and logs. |
| src/app/home/info/console-sidebar-panel.component.sass | Styling for sidebar panel cards/logs/action button. |
| src/app/home/info/console-sidebar-panel.component.html | Sidebar template: specs, logs, primary action, personal notes. |
| src/app/home/info/console-shell.data.ts | Centralized data for nav rail, focus, profile details. |
| src/app/home/info/console-project-registry.component.ts | Component for project registry cards + module links. |
| src/app/home/info/console-project-registry.component.sass | Styling for registry grid/cards/modules. |
| src/app/home/info/console-project-registry.component.html | Registry template with external/router links and placeholder. |
| src/app/home/info/console-profile-badge.component.ts | Badge component for avatar + status indicator. |
| src/app/home/info/console-profile-badge.component.spec.ts | Unit tests for badge rendering + status toggle. |
| src/app/home/info/console-profile-badge.component.sass | Badge styling and ping animation. |
| src/app/home/info/console-profile-badge.component.html | Badge template with optional status indicator. |
| src/app/home/info/console-pipeline-panel.component.ts | Pipeline panel component for capability stages. |
| src/app/home/info/console-pipeline-panel.component.sass | Styling for pipeline grid + progress tracks. |
| src/app/home/info/console-pipeline-panel.component.html | Pipeline template with tone classes and arrows. |
| src/app/home/info/console-left-rail.component.ts | Left rail navigation component. |
| src/app/home/info/console-left-rail.component.sass | Left rail styling and sticky behavior. |
| src/app/home/info/console-left-rail.component.html | Left rail template with primary + utility items. |
| src/app/home/info/console-hero-panel.component.ts | Hero panel component using GitHub user + i18n quote. |
| src/app/home/info/console-hero-panel.component.sass | Styling for hero card, actions, specs, and metrics. |
| src/app/home/info/console-hero-panel.component.html | Hero template with translated quote and primary actions. |
| src/app/home/home.module.ts | Wires new console/personal-info standalone components into Home. |
| src/app/home/home.component.ts | Minor metadata cleanup for Home component. |
| src/app/footer/footer.component.ts | Simplifies footer component now using routerLink in template. |
| src/app/footer/footer.component.spec.ts | Updates testbed and asserts curriculum link exists. |
| src/app/footer/footer.component.sass | New fixed console footer styling. |
| src/app/footer/footer.component.html | New footer markup with console status + links. |
| src/app/experience/viewer/viewer.component.ts | Adds console rail integration + document facts for viewer page. |
| src/app/experience/viewer/viewer.component.spec.ts | Adds tests for pagination/zoom and TranslateModule setup. |
| src/app/experience/viewer/viewer.component.sass | New console-styled viewer and meta panel layout. |
| src/app/experience/viewer/viewer.component.html | Replaces viewer layout with console shell + translated strings. |
| src/app/experience/experience.module.ts | Imports TranslateModule and ConsoleLeftRailComponent. |
| src/app/experience/experience.component.ts | Minor metadata cleanup for Experience component. |
| src/app/common/console/console.module.ts | Removes HttpClientModule import; streamlines module definition. |
| src/app/common/console/console.component.ts | Formatting + updated template semantics for degree/major/minor. |
| src/app/common/console/console.component.spec.ts | Updates tests; stubs CountryService and asserts rendered text. |
| src/app/common/console/console.component.html | Migrates to @if blocks; fixes prior string concatenation issue. |
| src/app/ball/ball.module.ts | New lazy-loaded module for WebGL demo route with i18n. |
| src/app/ball/ball.component.ts | Three.js WebGL demo with resize handling and cleanup. |
| src/app/ball/ball.component.spec.ts | Tests success path and WebGL failure fallback UI. |
| src/app/ball/ball.component.sass | Styling for full-screen canvas and fallback content. |
| src/app/ball/ball.component.html | Canvas vs fallback UI with translated strings. |
| src/app/ball/ball-routing.module.ts | Adds /ball feature routing. |
| src/app/app.module.ts | Switches to provideHttpClient and configures ngx-translate loader. |
| src/app/app.component.ts | Initializes i18n based on browser language. |
| src/app/app.component.spec.ts | Adds tests for i18n initialization and layout padding for footer. |
| src/app/app.component.html | Sets global background and reserves space for fixed footer. |
| src/app/app-routing.module.ts | Adds /ball route and enables anchor scrolling + scroll offset. |
| scripts/format-check-staged.mjs | Adds staged-file Prettier check script. |
| package.json | Dependency/tooling upgrades; adds format scripts and pre-commit hook. |
| karma.conf.js | Switches to karma-coverage reporter configuration. |
| angular.json | Updates Angular CLI config structure and formatting. |
| README.md | Documents personal info feature configuration and security note. |
| AGENTS.md | Adds repository contribution rules (commit hooks + conventional commits). |
| .prettierrc | Adds Prettier config (single quotes). |
| .github/workflows/node-ci.yml | Adds CI job for test + lint on pushes/PRs. |
| .github/workflows/firebase-hosting-pull-request.yml | Adds test/lint steps before PR preview deploy. |
| .github/workflows/firebase-hosting-merge.yml | Adds test/lint steps before merge deploy. |
| .github/workflows/firebase-hosting-cleanup.yml | Adds workflow to delete preview channels on PR close. |
| .eslintrc.json | Updates lint config formatting and disables standalone-related rules. |
| .coderabbit.yaml | Adds CodeRabbit auto-review configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Update workflow actions, harden shell and navigation guards, and tighten specs for Ball, services, and console behavior.
Integrate origin/main into develop, keep the current develop shell and workflow fixes, and clear the main-versus-develop merge blockers for PR 374.
|
Visit the preview URL for this PR (updated for commit 1c4536f): https://fardust--pr-374-mxquh5kp.web.app (expires Sat, 28 Mar 2026 07:19:38 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 0f6899045884acfb21b4b818dabc8dfdf9a83d8d |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/template.css (1)
23-35:⚠️ Potential issue | 🟡 MinorDrop quotes from single-word font families.
Stylelint's
font-family-name-quotesrule requires quotes only for multi-word font names. On lines 23 and 35,'Inter'should be unquoted while'Helvetica Neue'and'Inter Tight'remain quoted.Proposed fix
- font-family: 'Inter', 'Helvetica Neue', Helvetica, Arial, sans-serif; + font-family: Inter, 'Helvetica Neue', Helvetica, Arial, sans-serif; @@ font-family: - 'Inter Tight', 'Inter', 'Helvetica Neue', Helvetica, Arial, sans-serif; + 'Inter Tight', Inter, 'Helvetica Neue', Helvetica, Arial, sans-serif;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/template.css` around lines 23 - 35, The CSS uses quoted single-word font-family names which violates stylelint's font-family-name-quotes rule: update the font-family declarations so single-word names like 'Inter' are unquoted while keeping multi-word names like 'Helvetica Neue' and 'Inter Tight' quoted; locate the font-family lines (the global font-family and the heading font-family block) and remove quotes around Inter only (leave 'Helvetica Neue' and 'Inter Tight' quoted) so the declarations for font-family and the h1–h6 block conform to the rule.
🧹 Nitpick comments (4)
src/app/experience/viewer/viewer.component.spec.ts (1)
48-56: Weak assertion may mask floating-point precision issue.The test title states "respecting lower bound" (which is 0.5 per the implementation), but the assertion
toBeGreaterThan(0.39)allows values down to ~0.4. Due to floating-point arithmetic,0.6 - 0.1can yield0.5000000000000001, causing thezoom > 0.5check to pass and allow one extra decrement to ~0.4.Consider tightening the assertion to match the intended bound:
♻️ Proposed fix
it('should zoom in and out respecting lower bound', () => { const initial = component.zoom; component.zoomIn(); expect(component.zoom).toBeCloseTo(initial + 0.1, 5); for (let i = 0; i < 10; i++) { component.zoomOut(); } - expect(component.zoom).toBeGreaterThan(0.39); + expect(component.zoom).toBeCloseTo(0.5, 1); });Alternatively, if the actual behavior allows zoom to reach ~0.4, the implementation's bound check should use
>=instead of>to avoid floating-point drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/experience/viewer/viewer.component.spec.ts` around lines 48 - 56, The test currently allows zoom to drift below the intended lower bound; update the spec to assert the actual intended bound using component.zoom (after calling component.zoomIn()/component.zoomOut()) — for example replace the loose expect(component.zoom).toBeGreaterThan(0.39) with a tighter assertion that verifies the lower bound (e.g., expect(component.zoom).toBeGreaterThanOrEqual(0.5) or use toBeCloseTo(0.5, <appropriatePrecision>) ) so the test matches the implementation's 0.5 limit and accounts for floating-point precision..github/workflows/firebase-hosting-cleanup.yml (2)
17-17: Pinfirebase-toolsto a validated major/minor version for reproducible deployments.Installing the latest CLI on every run introduces behavior drift over time. The current stable version is
15.11.0; pinning ensures consistent cleanup behavior across runs.Suggested patch
- - run: npm install -g firebase-tools + - run: npm install -g firebase-tools@15.11.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/firebase-hosting-cleanup.yml at line 17, Pin the firebase-tools installation in the workflow step to a validated release to avoid drift: replace the install command that currently runs "npm install -g firebase-tools" with a pinned version such as "npm install -g firebase-tools@15.11.0" so the cleanup job always uses the known stable CLI; update any related workflow step name or comments if present (the run step installing firebase-tools).
18-20: Use the recommended CI-safe pattern for deleting preview channels in CI.The
firebase hosting:channel:deletecommand exits with code 1 when the channel doesn't exist (HTTP 404). This causes noisy workflow failures if the preview deploy was skipped. Instead of suppressing errors, check channel existence first, then conditionally delete:- run: | if firebase hosting:channel:list --project landing-fardust | grep -q "$FIREBASE_PREVIEW_CHANNEL_ID"; then firebase hosting:channel:delete "$FIREBASE_PREVIEW_CHANNEL_ID" --project landing-fardust --force fi env: FIREBASE_CLI_PREVIEWS: hostingchannelsThis avoids unnecessary API calls and errors for missing channels.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/firebase-hosting-cleanup.yml around lines 18 - 20, Replace the direct call to firebase hosting:channel:delete with a CI-safe check that first lists channels and only deletes if the channel exists: use firebase hosting:channel:list (for project landing-fardust) piped to grep -q "$FIREBASE_PREVIEW_CHANNEL_ID" and, if present, run firebase hosting:channel:delete "$FIREBASE_PREVIEW_CHANNEL_ID" --project landing-fardust --force; keep the FIREBASE_CLI_PREVIEWS: hostingchannels env var.src/app/home/info/console-profile-badge.component.sass (1)
77-95: Respect reduced-motion for the status pulse.The ping animation runs forever on an above-the-fold avatar. Please suppress it under
prefers-reduced-motion; the solid status dot already carries the state without the extra motion.Proposed fix
.console-profile-badge__status-ping top: 50% left: 50% width: calc(100% + 4px) height: calc(100% + 4px) opacity: 0.5 pointer-events: none animation: console-profile-badge-pulse 1.8s ease-out infinite +@media (prefers-reduced-motion: reduce) + .console-profile-badge__status-ping + display: none + `@keyframes` console-profile-badge-pulse🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/info/console-profile-badge.component.sass` around lines 77 - 95, The ping animation should be suppressed when the user prefers reduced motion: add a prefers-reduced-motion media rule that targets .console-profile-badge__status-ping and disables the animation (e.g., set animation: none and keep a static transform/opacity matching the initial state) so the status dot remains visible without pulsing; reference the existing .console-profile-badge__status-ping selector and `@keyframes` console-profile-badge-pulse when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/codeql-analysis.yml:
- Line 42: Replace the deprecated CodeQL action references that use `@v2` with the
supported v4 release: find occurrences of "uses: github/codeql-action/init@v2",
"uses: github/codeql-action/autobuild@v2", and "uses:
github/codeql-action/analyze@v2" and update them to "uses:
github/codeql-action/init@v4", "uses: github/codeql-action/autobuild@v4", and
"uses: github/codeql-action/analyze@v4" respectively; ensure no other arguments
need changing and run a workflow lint or test to confirm compatibility.
In @.github/workflows/firebase-hosting-cleanup.yml:
- Line 14: The workflow uses an outdated GitHub Action reference "uses:
google-github-actions/auth@v1" which is incompatible; update that action
reference to "google-github-actions/auth@v3" (or `@v2`) in the Firebase hosting
cleanup job so the runner can authenticate properly—locate the line containing
google-github-actions/auth in the workflow and replace the version tag from `@v1`
to `@v3`, ensuring any required inputs for the newer major version are adjusted if
your job uses new auth features.
In `@src/app/home/info/console-hero-panel.component.html`:
- Around line 27-33: The hero primary CTA currently navigates to /experience but
the copy/icon imply a PDF download; update the anchor in
console-hero-panel.component.html (the element with class
console-hero__action--primary) to either bind to the real CV URL (use
primaryAction.href from the parent/info component) and add download semantics
([href]="primaryAction.href" and download) and update the visible text/icon to
"Download CV" or, alternatively, change the label/icon to accurately reflect
navigation (e.g., "View Experience") if you intend to keep
routerLink="/experience"; reference primaryAction.href and the console-hero
action anchor to locate and apply the fix.
In `@src/app/home/info/info.component.html`:
- Around line 1-4: The duplicated full-screen gradient element should be removed
so the shared .background CSS can control the gradient and allow
.background::before to correctly overlay the grid; delete the inner div with the
classes "absolute inset-0
bg-[radial-gradient(circle_at_top_right,rgba(173,198,255,0.14),transparent_28%),linear-gradient(180deg,rgba(11,19,38,0.9),rgba(11,19,38,0.98))]"
(the inline gradient layer) and rely on the existing .background rule instead,
ensuring no other code reintroduces a full-screen absolute gradient that would
sit above .background::before.
---
Outside diff comments:
In `@src/template.css`:
- Around line 23-35: The CSS uses quoted single-word font-family names which
violates stylelint's font-family-name-quotes rule: update the font-family
declarations so single-word names like 'Inter' are unquoted while keeping
multi-word names like 'Helvetica Neue' and 'Inter Tight' quoted; locate the
font-family lines (the global font-family and the heading font-family block) and
remove quotes around Inter only (leave 'Helvetica Neue' and 'Inter Tight'
quoted) so the declarations for font-family and the h1–h6 block conform to the
rule.
---
Nitpick comments:
In @.github/workflows/firebase-hosting-cleanup.yml:
- Line 17: Pin the firebase-tools installation in the workflow step to a
validated release to avoid drift: replace the install command that currently
runs "npm install -g firebase-tools" with a pinned version such as "npm install
-g firebase-tools@15.11.0" so the cleanup job always uses the known stable CLI;
update any related workflow step name or comments if present (the run step
installing firebase-tools).
- Around line 18-20: Replace the direct call to firebase hosting:channel:delete
with a CI-safe check that first lists channels and only deletes if the channel
exists: use firebase hosting:channel:list (for project landing-fardust) piped to
grep -q "$FIREBASE_PREVIEW_CHANNEL_ID" and, if present, run firebase
hosting:channel:delete "$FIREBASE_PREVIEW_CHANNEL_ID" --project landing-fardust
--force; keep the FIREBASE_CLI_PREVIEWS: hostingchannels env var.
In `@src/app/experience/viewer/viewer.component.spec.ts`:
- Around line 48-56: The test currently allows zoom to drift below the intended
lower bound; update the spec to assert the actual intended bound using
component.zoom (after calling component.zoomIn()/component.zoomOut()) — for
example replace the loose expect(component.zoom).toBeGreaterThan(0.39) with a
tighter assertion that verifies the lower bound (e.g.,
expect(component.zoom).toBeGreaterThanOrEqual(0.5) or use toBeCloseTo(0.5,
<appropriatePrecision>) ) so the test matches the implementation's 0.5 limit and
accounts for floating-point precision.
In `@src/app/home/info/console-profile-badge.component.sass`:
- Around line 77-95: The ping animation should be suppressed when the user
prefers reduced motion: add a prefers-reduced-motion media rule that targets
.console-profile-badge__status-ping and disables the animation (e.g., set
animation: none and keep a static transform/opacity matching the initial state)
so the status dot remains visible without pulsing; reference the existing
.console-profile-badge__status-ping selector and `@keyframes`
console-profile-badge-pulse when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d8b53eb6-30e0-47c9-98f8-fe5fab09c480
📒 Files selected for processing (25)
.github/workflows/codeql-analysis.yml.github/workflows/firebase-hosting-cleanup.yml.github/workflows/firebase-hosting-merge.yml.github/workflows/firebase-hosting-pull-request.yml.github/workflows/node-ci.ymlsrc/app/ball/ball.component.spec.tssrc/app/ball/ball.component.tssrc/app/common/console/console.component.htmlsrc/app/common/console/console.component.spec.tssrc/app/experience/viewer/viewer.component.spec.tssrc/app/experience/viewer/viewer.component.tssrc/app/home/info/console-hero-panel.component.htmlsrc/app/home/info/console-hero-panel.component.tssrc/app/home/info/console-profile-badge.component.htmlsrc/app/home/info/console-profile-badge.component.sasssrc/app/home/info/console-profile-badge.component.spec.tssrc/app/home/info/console-profile-badge.component.tssrc/app/home/info/info.component.htmlsrc/app/home/info/info.component.spec.tssrc/app/navbar/navbar.component.spec.tssrc/app/navbar/navbar.component.tssrc/app/services/country.service.spec.tssrc/app/services/github.service.spec.tssrc/styles.sasssrc/template.css
✅ Files skipped from review due to trivial changes (3)
- .github/workflows/node-ci.yml
- src/app/home/info/console-profile-badge.component.spec.ts
- src/styles.sass
🚧 Files skipped from review as they are similar to previous changes (14)
- .github/workflows/firebase-hosting-merge.yml
- .github/workflows/firebase-hosting-pull-request.yml
- src/app/common/console/console.component.html
- src/app/navbar/navbar.component.spec.ts
- src/app/services/github.service.spec.ts
- src/app/common/console/console.component.spec.ts
- src/app/services/country.service.spec.ts
- src/app/ball/ball.component.spec.ts
- src/app/home/info/console-profile-badge.component.ts
- src/app/navbar/navbar.component.ts
- src/app/ball/ball.component.ts
- src/app/home/info/console-hero-panel.component.ts
- src/app/experience/viewer/viewer.component.ts
- src/app/home/info/info.component.spec.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 66 out of 67 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+67
to
+73
| <button | ||
| (click)="nextPage()" | ||
| [disabled]="totalPages && page >= totalPages" | ||
| class="experience-viewer__button" | ||
| > | ||
| {{ 'next' | translate }} | ||
| </button> |
There was a problem hiding this comment.
The nextPage() method now returns early when totalPages is not yet known, but the "Next" button stays enabled in that state ([disabled]="totalPages && page >= totalPages"). This results in a clickable button that does nothing until the PDF loads. Consider disabling the button when !totalPages (or adjusting nextPage()/the disabled condition so they match).
Comment on lines
+44
to
+50
| <div class="experience-viewer__toolbar"> | ||
| <div class="experience-viewer__chips"> | ||
| <span | ||
| class="experience-viewer__chip experience-viewer__chip--success" | ||
| > | ||
| Pdf Loaded | ||
| </span> |
There was a problem hiding this comment.
The toolbar shows a success chip "Pdf Loaded" unconditionally. Since the PDF load is asynchronous (after-load-complete sets totalPages), this status will be incorrect on initial render and if loading fails. Consider rendering this chip conditionally (e.g., only when totalPages is set) or changing the copy to indicate a pending/loading state until the load event fires.
Update workflow actions, align the hero CV action and shell background, and move the public ipinfo token into environment config.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/app/home/info/console-hero-panel.component.spec.ts (1)
6-37: Consider adding test coverage for fallback paths and basic component creation.The current test only verifies the happy path when
primaryAction.hrefis provided. TheprimaryActionHref()computed signal in the component has a fallback URL that remains untested.Missing coverage includes:
- A basic "should create" smoke test
- Fallback URL when
primaryAction.hrefis falsy/undefineddisplayName/displayInitialsfallback logic whengithubinput is null💡 Example test for fallback behavior
it('should create', () => { expect(component).toBeTruthy(); }); it('should use fallback href when primaryAction.href is undefined', () => { fixture.componentRef.setInput('primaryAction', { label: 'OPEN_CV', href: undefined, }); fixture.detectChanges(); const primaryAction: HTMLAnchorElement = fixture.nativeElement.querySelector('.console-hero__action--primary'); expect(primaryAction.getAttribute('href')).toBe( 'https://storage.googleapis.com/landing-artifacts/curriculum/cv-gabriel-faundez.pdf', ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/info/console-hero-panel.component.spec.ts` around lines 6 - 37, Add missing tests to cover basic creation and fallback paths: add a "should create" smoke test that asserts component is truthy; add a test for the primaryActionHref() fallback by calling fixture.componentRef.setInput('primaryAction', { label: 'OPEN_CV', href: undefined }) then fixture.detectChanges() and assert the anchor's href equals the fallback URL; add a test for displayName/displayInitials fallback by setting fixture.componentRef.setInput('github', null), fixture.detectChanges(), and asserting the component/template shows the expected displayName/displayInitials values; locate these changes in the ConsoleHeroPanelComponent spec and use fixture.componentRef.setInput, fixture.detectChanges(), and DOM queries to assert the fallback behaviors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/app/home/info/console-hero-panel.component.spec.ts`:
- Around line 6-37: Add missing tests to cover basic creation and fallback
paths: add a "should create" smoke test that asserts component is truthy; add a
test for the primaryActionHref() fallback by calling
fixture.componentRef.setInput('primaryAction', { label: 'OPEN_CV', href:
undefined }) then fixture.detectChanges() and assert the anchor's href equals
the fallback URL; add a test for displayName/displayInitials fallback by setting
fixture.componentRef.setInput('github', null), fixture.detectChanges(), and
asserting the component/template shows the expected displayName/displayInitials
values; locate these changes in the ConsoleHeroPanelComponent spec and use
fixture.componentRef.setInput, fixture.detectChanges(), and DOM queries to
assert the fallback behaviors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3e79a6dd-74e2-435c-828e-cc2103749d66
📒 Files selected for processing (10)
.github/workflows/codeql-analysis.yml.github/workflows/firebase-hosting-cleanup.ymlsrc/app/home/info/console-hero-panel.component.htmlsrc/app/home/info/console-hero-panel.component.spec.tssrc/app/home/info/console-hero-panel.component.tssrc/app/home/info/info.component.htmlsrc/app/services/country.service.spec.tssrc/app/services/country.service.tssrc/environments/environment.prod.tssrc/environments/environment.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/environments/environment.ts
- src/environments/environment.prod.ts
- .github/workflows/codeql-analysis.yml
- .github/workflows/firebase-hosting-cleanup.yml
- src/app/services/country.service.spec.ts
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
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 by CodeRabbit
Release Notes
New Features
Improvements