Repository navigation
Added skill and claude file - #116
Conversation
WalkthroughIntroduces a comprehensive "Warm & Friendly" design system redesign across the application. Adds a new ThemeService for dark/light/system theme management, a complete SCSS theme file with CSS variables for both modes, global stylesheet overhaul with warm color tokens, and styling updates to 80+ component files. Includes theme toggle integration in the navbar, documentation for the design system, and HTML initialization script for theme persistence and system preference detection. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (Page Load)
participant Script as Inline Theme Script
participant LocalStorage as localStorage
participant MediaQuery as System Preference
participant DOM as DOM Element
participant CSS as CSS Engine
Browser->>Script: Execute theme init script
Script->>LocalStorage: Check saved theme preference
alt Theme saved in localStorage
LocalStorage-->>Script: Return saved theme
else No saved preference
Script->>MediaQuery: Check system color scheme
MediaQuery-->>Script: Return system preference
end
Script->>DOM: Set data-theme attribute
DOM->>CSS: Apply CSS vars for current theme
CSS-->>Browser: Render page with theme colors
Note over Browser,CSS: User sees light/dark mode immediately<br/>(FOUC prevention via no-transitions class)
rect rgba(100, 150, 200, 0.5)
Note over Browser,CSS: Runtime Theme Toggle
Browser->>NavbarComponent: Click theme toggle button
NavbarComponent->>ThemeService: Call toggleTheme()
ThemeService->>ThemeService: Update theme signal
ThemeService->>LocalStorage: Persist new theme
ThemeService->>DOM: Update data-theme attribute
DOM->>CSS: Apply new theme CSS variables
CSS-->>Browser: Transition to new theme
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes The PR encompasses substantial changes across 150+ files with a mix of new complex logic (ThemeService with signal management and system preference detection) and extensive homogeneous styling updates. While the repetitive styling pattern reduces individual file complexity, the sheer scope, diverse file types, careful integration points (theme initialization, navbar integration, global styles), and new service logic requiring attention to localStorage/media query handling collectively demand comprehensive review. Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/app/modules/admin/components/telegram/salaries-stat-subscriptions/stat-data-cache-records.component.html (1)
37-57: Use<button>for the professions modal trigger (semantic + accessibility).
<a href="#">withpreventDefault()is non-semantic for action-only controls. Use<button type="button">to ensure proper keyboard navigation, screen reader support, and semantic correctness.🛠️ Suggested fix
-<a - *ngIf="item.professionIds && item.professionIds.length > 0" - href="#" - class="text-primary" - (click)=" - openProfessionsModal(item.professionIds); - $event.preventDefault() - " -> +<button + *ngIf="item.professionIds && item.professionIds.length > 0" + type="button" + class="btn btn-link p-0 text-primary" + (click)="openProfessionsModal(item.professionIds)" +> {{ item.professionIds.length }} специальност{{ item.professionIds.length === 1 ? "ь" : item.professionIds.length < 5 ? "и" : "ей" }} -</a> +</button>src/app/modules/admin/components/telegram/job-posting-message-subscriptions/job-posting-message-subscriptions.component.html (1)
32-39: Prefer a button for the modal action.
Usinghref="#"withpreventDefault()is less accessible than a real button.♻️ Suggested change
- <a + <button *ngIf="item.professionIds && item.professionIds.length > 0" - href="#" - class="text-primary" - (click)=" - openProfessionsModal(item.professionIds); - $event.preventDefault() - " + type="button" + class="btn btn-link p-0 text-primary" + (click)="openProfessionsModal(item.professionIds)" > {{ item.professionIds.length }} специальност{{ item.professionIds.length === 1 ? "ь" : item.professionIds.length < 5 ? "и" : "ей" }} - </a> + </button>src/app/modules/salaries/components/salaries-chart/salaries-adding-chart/salaries-adding-chart.component.scss (1)
14-22: Fix container ID spelling in CSS selector.Line 14 uses
#canvas-addint-trend-chart-container, but the template (line 3 of the HTML file) usesid="canvas-adding-trend-chart-container". The styles won't apply due to this mismatch. Change the selector to match the template:Fix
-#canvas-addint-trend-chart-container { +#canvas-adding-trend-chart-container {src/app/modules/admin/components/telegram/job-posting-message-subscriptions/job-posting-message-subscriptions.component.scss (1)
71-83: Retain horizontal scrolling on responsive tables.Setting
.table-responsive { overflow: visible; }disables Bootstrap’s horizontal scroll and can break on narrow screens. Preferoverflow-x: autowhile keepingoverflow-yvisible for dropdowns.🛠️ Proposed fix
.table-responsive { - overflow: visible; + overflow-x: auto; + overflow-y: visible; .dropdown-menu { position: absolute;src/app/modules/admin/components/telegram/salaries-stat-subscriptions/stat-data-cache-records.component.scss (1)
82-93: Retain horizontal scrolling on responsive tables.Setting
.table-responsive { overflow: visible; }disables Bootstrap’s horizontal scroll and can break on narrow screens. Preferoverflow-x: autowhile keepingoverflow-yvisible for dropdowns.🛠️ Proposed fix
.table-responsive { - overflow: visible; + overflow-x: auto; + overflow-y: visible; .dropdown-menu { position: absolute;src/app/components/admin-navbar/admin-navbar.component.html (2)
17-27: Duplicateid="navbarDropdown"creates accessibility and validity issues.Each dropdown in the
*ngForloop gets the sameid="navbarDropdown", which violates HTML specification (IDs must be unique) and breaks accessibility sincearia-labelledbyreferences become ambiguous.🐛 Proposed fix - use unique IDs
- <li class="nav-item dropdown" *ngFor="let dropdown of dropdowns"> + <li class="nav-item dropdown" *ngFor="let dropdown of dropdowns; let i = index; trackBy: trackByTitle"> <a class="nav-link dropdown-toggle" href="#" - id="navbarDropdown" + [id]="'adminNavbarDropdown-' + i" role="button" data-bs-toggle="dropdown" aria-expanded="false" > {{ dropdown.title }} </a> - <ul class="dropdown-menu" aria-labelledby="navbarDropdown"> + <ul class="dropdown-menu" [attr.aria-labelledby]="'adminNavbarDropdown-' + i">Add trackBy function in the component:
trackByTitle(index: number, item: { title: string }): string { return item.title; }
30-37: Addrel="noopener noreferrer"to external links for security.External links with
target="_blank"should includerel="noopener noreferrer"to prevent the opened page from accessingwindow.opener(reverse tabnabbing vulnerability).🔒 Proposed fix
<a *ngIf="link.isExternal" class="dropdown-item" href="{{ link.url }}" target="_blank" + rel="noopener noreferrer" >{{ link.title }} <i class="bi bi-box-arrow-up-right ms-1 small"></i> </a>
🤖 Fix all issues with AI agents
In `@CLAUDE.md`:
- Around line 31-44: The fenced code block containing the directory tree in
CLAUDE.md is missing a language tag which triggers markdownlint MD040; edit the
opening fence for that block and add the language identifier text (i.e., change
the triple backtick fence that begins the directory tree to ```text) so the
block is properly annotated. Ensure you only update the opening fence and leave
the directory tree contents unchanged.
In `@src/_warm-theme.scss`:
- Around line 57-110: Define the missing shadow-size CSS variables
(--warm-shadow-sm, --warm-shadow-md, --warm-shadow-lg, --warm-shadow-xl) in the
same :root / theme block so components using var(--warm-shadow-*) render; create
them as box-shadow strings that reference the existing color variables
(--warm-shadow-color, --warm-shadow-color-medium, --warm-shadow-color-strong)
and appropriate offsets/blur (e.g., small: subtle 0 1px 2px, medium: 0 4px 8px,
large: 0 10px 20px, xl: 0 20px 40px) so components like those expecting
--warm-shadow-sm / --warm-shadow-md / --warm-shadow-lg / --warm-shadow-xl
receive concrete box-shadow values.
In
`@src/app/modules/admin/components/openai-prompts/openai-prompts-admin-page.component.scss`:
- Around line 31-37: The .text-truncate SCSS max-width (in OpenAI Prompts admin
styles) conflicts with an inline style in the component template; update the
styling to be consistent by either changing .text-truncate's max-width to 210px
and removing the inline style from the OpenaiPromptsAdminPage component
template, or introduce a modifier class (e.g., .text-truncate--sm { max-width:
210px }) and replace the inline style with that class in the template so the
CSS, not inline styles, controls the width.
In
`@src/app/modules/salaries/components/salaries-chart/salaries-chart.component.html`:
- Around line 524-526: The app-data-loader is rendered with an empty title which
removes context for screen readers; update the usage in
salaries-chart.component.html to pass a meaningful, localized loading string
into the [title] input (or add an aria-label) instead of an empty string — e.g.,
provide a localized key like "salaries.loading" or call the component's
i18n/gettext helper and bind it to [title] (or add aria-label="...") so screen
readers receive descriptive loading text when app-data-loader is visible.
In `@src/app/shared/components/dialogs/dialog/dialog.component.scss`:
- Around line 1-2: The global modal styles lack mobile-responsive rules—update
the global styles (where .modal is defined in styles.scss and theme tokens in
_warm-theme.scss) to add `@media` queries for small viewports that reduce modal
width/max-width, decrease padding, set a sensible max-height with overflow:auto
or -webkit-overflow-scrolling:touch, and ensure border-radius and animations
still work with CSS variables; adjust any theme variable fallbacks in
_warm-theme.scss if needed and remove or leave dialog.component.scss empty since
component-specific overrides aren’t required.
In `@src/app/shared/services/theme/theme.service.ts`:
- Around line 124-140: The initSystemPreferenceDetection method adds a
MediaQueryList listener (this.mediaQuery) via addEventListener/addListener but
never removes it; update the service to store the handler (e.g., a private
property like this._mqHandler or reuse the existing handler reference) and
implement cleanup by removing the listener on destroy: implement OnDestroy on
the service and in ngOnDestroy call
this.mediaQuery.removeEventListener("change", handler) with a fallback to
this.mediaQuery.removeListener(handler) for older browsers, ensuring the same
handler reference is used so removal succeeds.
In `@src/index.html`:
- Around line 73-99: Wrap the localStorage access in a try/catch around the
localStorage.getItem("theme") call so restricted storage errors don’t throw;
when an exception occurs, treat savedTheme as null (fall back to
systemPrefersDark detection) and proceed to set
document.documentElement.setAttribute("data-theme", "dark") only when savedTheme
=== "dark" or savedTheme is null and systemPrefersDark is true, otherwise set
"light" only when savedTheme === "light"; keep the window.load listener and
no-transitions removal unchanged.
🧹 Nitpick comments (64)
src/app/modules/salaries/components/people-by-grades-chart/people-by-grades-chart.component.scss (1)
16-25: Add keyboard focus styling to match hover.
.progresslooks interactive (cursor: pointer) but only defines hover feedback. Please ensure a visible focus state for keyboard users.♿ Suggested focus-visible enhancement
.progress { min-height: 30px; cursor: pointer; background-color: var(--warm-bg-tertiary); border-radius: 8px; overflow: hidden; + transition: box-shadow 0.2s ease; - &:hover { + &:hover, + &:focus-visible { box-shadow: 0 2px 8px var(--warm-shadow-color); } }src/app/modules/home/components/privacy-policy-page/privacy-policy-page.component.html (1)
10-10: Consider using semantic heading tags for accessibility.The section headings use
<div class="h3">instead of semantic<h3>tags. While visually identical, semantic headings enable screen reader users to navigate the document by heading structure. As per coding guidelines for accessibility in Angular templates, consider using proper heading elements.♻️ Suggested refactor (example for one heading)
- <div class="mt-3 h3">1. Общие положения</div> + <h3 class="mt-3">1. Общие положения</h3>Apply similar changes to other section headings throughout the file.
Also applies to: 17-17
src/app/modules/admin/components/companies/companies-admin-page/companies-admin-page.component.html (4)
37-39: Addtype="button"for consistency.The adjacent buttons (lines 21–29 and 30–36) specify
type="button". Adding it here maintains consistency and is defensive practice.Suggested fix
- <button class="btn btn-primary ms-2" (click)="create()"> + <button type="button" class="btn btn-primary ms-2" (click)="create()"> Добавить компанию </button>
64-64: Add atrackByfunction for the*ngForloop.Without
trackBy, Angular re-renders all rows whencompanieschanges (e.g., on pagination or search). Track byitem.idto improve rendering performance. As per coding guidelines, trackBy functions should be used for ngFor loops.Suggested fix
In the component class, add:
trackByCompanyId(index: number, item: Company): number { return item.id; }Then update the template:
- <tr *ngFor="let item of companies; let i = index" class="small"> + <tr *ngFor="let item of companies; let i = index; trackBy: trackByCompanyId" class="small">
135-157: Associate labels with their inputs for accessibility.Screen readers rely on the
for/idassociation to announce form fields correctly. Each<label>should have aforattribute matching the corresponding input'sid. As per coding guidelines, accessibility attributes and semantic HTML should be reviewed.Example fix for the "Название" field
<div class="mb-3"> - <label>Название</label> - <input type="text" class="form-control" formControlName="name" /> + <label for="companyName">Название</label> + <input type="text" class="form-control" formControlName="name" id="companyName" /> <app-field-error [field]="editForm.get('name')"></app-field-error> </div>Apply the same pattern to the other fields (
description,socialLink,logoUrl).
9-18: Add an accessible label for the search input.The search input relies solely on
placeholdertext, which disappears when typing and isn't announced consistently by screen readers. Add a visually-hidden label oraria-labelfor accessibility. As per coding guidelines, accessibility attributes should be reviewed.Option: Use aria-label
<input type="text" placeholder="Поиск по названию компании. Минимум {{ MIN_SEARCH_QUERY_LENGTH }} символа" class="form-control" id="searchQuery" + aria-label="Поиск по названию компании" [(ngModel)]="searchQuery" (keyup)="onKeyupEvent($event)" />src/app/modules/salaries/components/salaries-chart/salary-block-value/salary-block-value.component.scss (1)
1-6: Consider a reduced‑motion fallback for the hover scale.Adds motion that some users may disable. A small
prefers-reduced-motionoverride keeps things accessible without changing default behavior.♿ Suggested tweak
.cat-image { transition: transform 0.3s ease; &:hover { transform: scale(1.1); } } + +@media (prefers-reduced-motion: reduce) { + .cat-image { + transition: none; + + &:hover { + transform: none; + } + } +}src/app/modules/salaries/components/edit-salary/edit-salary.component.scss (1)
10-22: Add a focus-visible state if this wrapper is interactive.Cursor implies clickability; mirroring hover styles on
:focus-visibleimproves keyboard accessibility.✅ Suggested focus style
.salary-value-wrapper { cursor: pointer; padding: 0.5rem 1rem; border-radius: 8px; background-color: var(--warm-bg-accent); border: 1px solid var(--warm-border-color); transition: all 0.2s ease; &:hover { background-color: var(--warm-accent-primary); border-color: var(--warm-accent-primary); color: white; } + + &:focus-visible { + background-color: var(--warm-accent-primary); + border-color: var(--warm-accent-primary); + color: white; + outline: 2px solid var(--warm-accent-primary); + outline-offset: 2px; + } }src/app/modules/interviews/components/public-interview-templates/public-interview-templates.component.scss (1)
54-141: Add keyboard focus styling for interactive template cards (if clickable).The hover state and pointer cursor imply interactivity; consider matching that with a
:focus-visiblestyle for keyboard users. As per coding guidelines, ...♿ Suggested focus-visible treatment
.template-card { background-color: var(--warm-card-bg); border-radius: 12px; padding: 1.5rem; border: 1px solid var(--warm-border-color); box-shadow: var(--warm-shadow-sm); transition: all 0.2s ease; cursor: pointer; &:hover { transform: translateY(-4px); box-shadow: var(--warm-shadow-lg); border-color: var(--warm-accent-primary); } + &:focus-visible { + transform: translateY(-4px); + box-shadow: var(--warm-shadow-lg); + border-color: var(--warm-accent-primary); + outline: 2px solid var(--warm-accent-primary); + outline-offset: 2px; + }src/app/modules/admin/components/admin-start-page/admin-start-page.component.scss (1)
53-63: Add focus-visible styling for.admin-link.Hover-only states make keyboard focus hard to see. Add
:focus-visibleto improve accessibility.🛠️ Suggested tweak
.admin-link { color: var(--warm-accent-primary); text-decoration: none; font-weight: 500; transition: color 0.2s ease; - &:hover { + &:hover, + &:focus-visible { color: var(--warm-accent-primary-hover); text-decoration: underline; + outline: 2px solid var(--warm-accent-primary); + outline-offset: 2px; } }Please verify keyboard focus visibility in the browser.
src/app/modules/interviews/components/my-interviews/my-interviews.component.scss (1)
84-107: Tokenize status background colors for theming consistency.The RGBA literals may not adapt well to dark mode. Prefer CSS variables (with fallback if needed).
🛠️ Suggested tweak
&.status-draft { - background-color: rgba(122, 122, 122, 0.15); + background-color: var(--warm-status-draft-bg, rgba(122, 122, 122, 0.15)); color: var(--warm-text-muted); } &.status-completed { - background-color: rgba(129, 178, 154, 0.15); + background-color: var(--warm-status-completed-bg, rgba(129, 178, 154, 0.15)); color: var(--warm-success); } &.status-in-progress { - background-color: rgba(224, 122, 95, 0.15); + background-color: var(--warm-status-in-progress-bg, rgba(224, 122, 95, 0.15)); color: var(--warm-accent-primary); }Please verify the status chip backgrounds in both light and dark themes.
src/app/app.component.scss (1)
24-75: Respectprefers-reduced-motionfor transitions and spinner.Add a reduced-motion override to avoid forcing animations for users who opt out.
🛠️ Suggested tweak
.page-enter-active { opacity: 1; transform: translateY(0); transition: opacity 0.3s ease, transform 0.3s ease; } @@ `@keyframes` spin { to { transform: rotate(360deg); } } + +@media (prefers-reduced-motion: reduce) { + .page-enter, + .page-enter-active { + transition: none; + transform: none; + } + + .global-loading .spinner { + animation: none; + } +}Please verify behavior with reduced-motion enabled at OS level.
src/app/modules/salaries/components/currencies-chart-page/currencies-chart-page.component.html (2)
13-21: Improve loading button accessibility.Add
aria-busyon the button and hide the decorative spinner from screen readers. As per coding guidelines, accessibility should be explicit in templates.♿ Proposed update
- <button + <button class="btn btn-sm btn-outline-secondary" (click)="refresh()" [disabled]="isLoading" + [attr.aria-busy]="isLoading" > <span *ngIf="isLoading" class="spinner-border spinner-border-sm me-1" + aria-hidden="true" ></span>
37-40: Consider extracting the no‑data condition into a getter.The multi-clause
*ngIfis readable but verbose; ahasNoDatagetter keeps the template lean. As per coding guidelines, avoid complex expressions in templates..claude/skills/frontend-design/SKILL.md (1)
40-42: Consider replacing the cliché phrase.A more specific phrase reads stronger in professional guidance.
✍️ Suggested wording
-Remember: Claude is capable of extraordinary creative work. Don't hold back, show what can truly be created when thinking outside the box and committing fully to a distinctive vision. +Remember: Claude is capable of extraordinary creative work. Don't hold back—show what can truly be created by breaking conventional patterns and committing fully to a distinctive vision.src/app/modules/admin/components/salaries/historical-data-templates/historical-data-templates.component.html (4)
28-32: Reduce work inside the items loop.Consider adding
trackByand precomputing the short ID (or using a pure pipe) instead ofsplit()in the template. As per coding guidelines, keep templates free of complex logic.♻️ Suggested template updates
- <tr class="small" *ngFor="let item of items; let i = index"> + <tr class="small" *ngFor="let item of items; trackBy: trackByItemId"> ... - <span class="text-muted small" [title]="item.id">...{{ item.id.split("-").pop() }}</span> + <span class="text-muted small" [title]="item.id">...{{ item.shortId }}</span>
45-51: Russian pluralization is incorrect for some counts.The
1 / 2–4 / elserule fails for 11–14, 21, 31, etc. Consideri18nPluralorIntl.PluralRuleswith a proper mapping. As per coding guidelines, ensure i18n correctness.Example approach:
{{ item.professionIds.length | i18nPlural: professionPluralMap }}
36-43: Use buttons for actions instead ofhref="#"links.This avoids unnecessary
preventDefault()and improves semantics/accessibility. As per coding guidelines, prefer accessible, semantic controls.♿ Example pattern (apply to similar links)
- <a - *ngIf="item.professionIds && item.professionIds.length > 0" - href="#" - class="text-primary" - (click)=" - openProfessionsModal(item.professionIds); - $event.preventDefault() - " - > + <button + *ngIf="item.professionIds && item.professionIds.length > 0" + type="button" + class="btn btn-link p-0 text-primary" + (click)="openProfessionsModal(item.professionIds)" + >Also applies to: 78-83
103-106: Remove the emptyclassattribute.It adds noise and can confuse future styling changes.
src/app/modules/admin/components/salaries/historical-data-templates/historical-data-templates.component.scss (1)
31-77:::ng-deepis deprecated; prefer global theming or library hooks.If possible, move ng-select theming into a global stylesheet or use ng-select’s built-in theming APIs to avoid deep selectors. As per coding guidelines,
::ng-deepshould be minimized.src/app/modules/admin/components/telegram/job-posting-message-subscriptions/job-posting-message-subscriptions.component.html (1)
41-47: Move pluralization logic to a helper/pipe.
The nested ternary hurts readability; consider a method or i18n plural pipe for Russian suffixes.src/app/modules/companies/components/companies-page/companies-page.component.scss (1)
59-73: Prefer explicit transitions overtransition: all.Line 60 will animate any changed property, which can cause unnecessary repaints. Limit to the properties you actually change on hover.
♻️ Proposed refactor
-.company-list-item { - transition: all 0.25s ease; +.company-list-item { + transition: transform 0.25s ease, box-shadow 0.25s ease, border-color 0.25s ease;src/_warm-theme.scss (1)
370-383: Respectprefers-reduced-motionfor theme transitions.Line 374 applies transitions globally; consider disabling them for users who prefer reduced motion.
♿ Suggested improvement
.no-transitions *::after { transition: none !important; } + +@media (prefers-reduced-motion: reduce) { + *, + *::before, + *::after { + transition: none !important; + } +}src/app/modules/admin/components/health-check-table/health-check-table.component.scss (1)
11-17: Add focus styling for keyboard users on clickable rows.Line 15 only styles hover. Consider
:focus-withinso keyboard focus inside the row gets the same affordance.♿ Suggested improvement
.clickable tr { cursor: pointer; transition: background-color 0.15s ease; - &:hover { + &:hover, + &:focus-within { background-color: var(--warm-bg-accent); } }src/app/modules/admin/components/generate-qr-code-page/generate-qr-page.component.scss (1)
42-58: Use the theme focus-ring token instead of hard-coded RGBA.Line 57 hard-codes the focus ring color; using
--warm-input-focus-ringkeeps it consistent across light/dark updates.♻️ Proposed refactor
- box-shadow: 0 0 0 3px rgba(224, 122, 95, 0.15); + box-shadow: 0 0 0 3px var(--warm-input-focus-ring);As per coding guidelines, prefer CSS variables for theme consistency.
src/app/modules/salaries/components/historical-charts-page/historical-charts-page.component.scss (1)
35-39: Use the theme heading font token instead of hard-coded font.Line 36 hard-codes
"Lora". Prefervar(--warm-font-heading)so it tracks theme updates.♻️ Proposed refactor
-.historical-title { - font-family: "Lora", Georgia, serif; +.historical-title { + font-family: var(--warm-font-heading);As per coding guidelines, prefer CSS variables for theme consistency.
src/app/modules/users/components/user-page/user-page.component.scss (1)
246-254: Hardcoded rgba colors break theme consistency.Lines 247 and 252 use hardcoded
rgba()values instead of CSS variables, which won't adapt to dark/light theme changes like other themed properties do.♻️ Suggested fix using CSS variables
&.premium { - background-color: rgba(242, 204, 143, 0.2); + background-color: var(--warm-accent-tertiary-bg, rgba(242, 204, 143, 0.2)); color: var(--warm-accent-tertiary); } &.verified { - background-color: rgba(129, 178, 154, 0.15); + background-color: var(--warm-success-bg, rgba(129, 178, 154, 0.15)); color: var(--warm-success); }Alternatively, define these in your
_warm-theme.scssas--warm-badge-premium-bgand--warm-badge-verified-bgwith appropriate values for both light and dark modes.src/app/modules/home/components/unsubscribe-me/unsubscribe-me.component.scss (1)
10-15: Hardcoded font-family should use CSS variable.Line 11 uses a hardcoded
"Nunito"font-family, while other components in this PR (e.g., user-page.component.scss) usevar(--warm-font-heading)for consistency.♻️ Suggested fix
.subtitle { - font-family: "Nunito", sans-serif; + font-family: var(--warm-font-heading); font-size: 1.5rem; font-weight: 400; color: var(--warm-text-secondary); }src/app/modules/salaries/components/people-by-age-chart/people-by-age-chart.component.scss (1)
18-32: Consider using a class selector instead of ID.Using
#canvas-containercreates high specificity that can make overrides difficult. In Angular component styles, class selectors are preferred for maintainability.♻️ Suggested change
-#canvas-container { +.canvas-container { position: relative; width: 100%; // ... }Update the template to use
class="canvas-container"accordingly.src/app/modules/interviews/components/interview-page/interview-page.component.scss (1)
14-47: Consider adding responsive breakpoints for mobile.The header section has fixed padding and font sizes that may not adapt well on smaller screens. Other chart components in this PR include
@media (max-width: 768px)queries.♻️ Example responsive adjustment
.interview-header { background-color: var(--warm-card-bg); border-radius: 16px; padding: 2rem; margin-bottom: 2rem; box-shadow: var(--warm-shadow-md); border: 1px solid var(--warm-border-color); + + `@media` (max-width: 768px) { + padding: 1.25rem; + border-radius: 12px; + } .interview-title { font-family: var(--warm-font-heading); font-size: 1.75rem; font-weight: 700; color: var(--warm-text-heading); margin-bottom: 0.5rem; + + `@media` (max-width: 768px) { + font-size: 1.5rem; + } }src/app/modules/interviews/components/interview-template-page/interview-template-page.component.scss (1)
14-53: Missing responsive breakpoints for header and stats sections.Similar to the interview-page component, this template page would benefit from responsive adjustments for padding, font sizes, and the fixed 40px stat-icon dimensions on mobile devices.
Also applies to: 104-148
src/app/modules/salaries/components/salaries-by-cities-chart/salaries-by-city-chart.component.scss (1)
18-26: Same ID selector pattern as other chart components.Consider using
.canvas-containerclass instead of#canvas-containerfor lower specificity and easier maintainability, consistent with Angular component styling best practices.src/app/modules/salaries/components/people-by-experience-chart/people-by-experience-chart.component.scss (1)
1-44: Consistent styling with sibling chart components.This file follows the same structure and theming patterns as the other chart components (people-by-age, salaries-by-cities), which promotes maintainability. The warm theme variables and responsive breakpoints are correctly applied.
The same optional suggestion applies: consider class selectors over ID selectors for
#canvas-containeracross all chart components for consistency with Angular best practices.src/app/modules/admin/components/telegram/job-posting-message-subscriptions/job-posting-message-subscriptions.component.scss (1)
24-68: Avoid long-term reliance on ::ng-deep for ng-select theming.
::ng-deepis deprecated and may break with future Angular versions. Consider moving these overrides into a global stylesheet or a shared theming layer for ng-select. As per coding guidelines, please keep ::ng-deep usage minimal and documented.src/app/modules/admin/components/telegram/salaries-stat-subscriptions/stat-data-cache-records.component.scss (1)
33-78: Avoid long-term reliance on ::ng-deep for ng-select theming.
::ng-deepis deprecated and may break with future Angular versions. Consider moving these overrides into a global stylesheet or shared theme layer for ng-select. As per coding guidelines, please keep ::ng-deep usage minimal and documented.src/app/modules/admin/components/users/users-admin-page/users-admin-page.component.html (2)
69-69: Add atrackByfunction to improve rendering performance.The
*ngForiterating overusersshould include atrackByfunction to help Angular efficiently track items and minimize DOM re-renders during pagination or filter changes.Suggested fix
In the component TypeScript, add:
trackByUserId(index: number, user: any): number { return user.id; }Then update the template:
-<tr *ngFor="let item of users; let i = index" class="small"> +<tr *ngFor="let item of users; let i = index; trackBy: trackByUserId" class="small">As per coding guidelines for Angular templates.
152-154: Add atrackByfunction for the options loop.The
*ngForiterating overoptionsshould include atrackByfunction for optimal change detection.Suggested fix
-<option *ngFor="let option of options" [value]="option.item"> +<option *ngFor="let option of options; trackBy: trackByOptionItem" [value]="option.item">As per coding guidelines for Angular templates.
src/app/modules/salaries/components/salaries-by-age-chart/salaries-by-age-chart.component.scss (1)
14-23: Consider using a CSS variable forborder-radiusfor consistency.The warm theme uses
12pxborder-radius here and in other components. For easier theme-wide adjustments, consider defining a--warm-border-radiusor similar token.Suggested approach
`#canvas-container` { position: relative; width: 100%; height: 100%; min-height: 250px; background-color: var(--warm-card-bg); - border-radius: 12px; + border-radius: var(--warm-border-radius-lg, 12px); padding: 1rem; }src/app/modules/interviews/components/interview-edit-page/interview-edit-page.component.scss (2)
23-26: Misleading class name:.w-10sets width to 20%.The class name suggests 10% width, but it actually sets
width: 20%. Consider renaming to.w-20for clarity, or use a utility class from Bootstrap if available.Suggested fix
-.w-10 { +.w-20 { width: 20%; max-width: 20%; }
73-77: Hardcodedrgbacolor may not adapt to dark mode.The focus box-shadow uses a hardcoded
rgba(224, 122, 95, 0.15). Consider using a CSS variable (e.g.,--warm-accent-focus-shadow) to ensure proper theming in both light and dark modes.Suggested approach
&:focus { border-color: var(--warm-accent-primary); - box-shadow: 0 0 0 3px rgba(224, 122, 95, 0.15); + box-shadow: 0 0 0 3px var(--warm-accent-focus-ring, rgba(224, 122, 95, 0.15)); outline: none; }Define
--warm-accent-focus-ringin the warm theme file with appropriate values for light/dark modes.src/app/modules/admin/components/companies/company-admin-page/company-admin-page.component.scss (1)
6-90: Good use of CSS variables for theming.The stylesheet properly uses
:hostfor component encapsulation and consistently applies--warm-*CSS variables for colors, fonts, and shadows, which aligns well with the design system approach. No deprecated::ng-deepusage.Optional: Consider extracting magic numbers like
border-radius: 12pxandborder-radius: 10pxinto CSS variables (e.g.,--warm-radius-lg,--warm-radius-md) for consistency across components, if not already defined in_warm-theme.scss.src/app/modules/admin/components/openai-prompts/openai-prompts-admin-page.component.html (3)
29-29: Add trackBy function for the ngFor loop.Per coding guidelines,
*ngForloops should include atrackByfunction to improve rendering performance when the list updates.Suggested implementation
In the component TypeScript:
trackByPromptId(index: number, item: any): number { return item.id; }In the template:
-<tr *ngFor="let item of prompts" class="small"> +<tr *ngFor="let item of prompts; trackBy: trackByPromptId" class="small">
32-34: Move inline style to SCSS.The
max-width: 210pxinline style should be defined in the component's SCSS file for maintainability and to avoid style duplication.
45-74: Consider adding aria-label for icon-only buttons.The action buttons use icons without visible text. While
titleattributes provide tooltips, addingaria-labelimproves screen reader accessibility.Example for one button
<button *ngIf="!item.isActive" class="btn btn-sm btn-outline-success me-1" (click)="activate(item)" title="Активировать" + aria-label="Активировать" >src/app/modules/salaries/components/historical-charts-page/historical-charts-page.component.html (1)
12-15: Add trackBy function for the ngFor loop.Per coding guidelines, the
*ngFordirective should include atrackByfunction to optimize DOM updates when the templates array changes.Suggested implementation
In the component TypeScript:
trackByTemplateName(index: number, template: any): string { return template.name || index.toString(); }In the template:
<div class="card card-body mobile-px-5 mobile-py-5 mb-4" - *ngFor="let template of data.templates" + *ngFor="let template of data.templates; trackBy: trackByTemplateName" >src/app/modules/admin/components/telegram/reviews-stat-subscriptions/reviews-stat-subscriptions.component.scss (1)
41-75: Consider extracting shared table styles.This table styling pattern is duplicated across multiple admin components (here and in
openai-prompts-admin-page.component.scss). Consider extracting into a shared mixin or utility class in_warm-theme.scssto reduce duplication.src/app/modules/salaries/components/salaries-chart/salary-chart-global-filters/salary-chart-global-filters.component.scss (1)
43-47: Use CSS variable instead of hardcoded color.The
color: whiteon line 46 breaks the theming pattern. Consider using a CSS variable like--warm-text-on-accentor similar to maintain theme consistency, especially for dark mode support.Suggested fix
&.active { background-color: var(--warm-accent-primary); border-color: var(--warm-accent-primary); - color: white; + color: var(--warm-text-on-accent, `#ffffff`); }src/app/modules/companies/components/add-review-page/add-review-page.component.scss (3)
45-102: Step indicator may overflow on narrow screens.
Consider wrapping and tightening connectors for small viewports.♻️ Suggested tweak
.step-indicator { display: flex; justify-content: center; gap: 1rem; margin-bottom: 2rem; + flex-wrap: wrap; + row-gap: 0.75rem; + `@media` (max-width: 600px) { + .step-connector { + flex: 1 1 24px; + width: 24px; + } + } }
158-178: Add keyboard focus styling to rating stars.
Hover-only feedback can miss keyboard users; add:focus-visible(and ensure focusable elements in the template).♿ Suggested focus style
.star { font-size: 1.5rem; cursor: pointer; color: var(--warm-border-color); transition: color 0.15s ease, transform 0.15s ease; + + &:focus-visible { + outline: 2px solid var(--warm-accent-primary); + outline-offset: 2px; + }
186-193: Actions row should wrap on small screens.
Allow the actions to stack to avoid overflow.♻️ Suggested tweak
.form-actions { margin-top: 2rem; display: flex; gap: 1rem; justify-content: flex-end; padding-top: 1.5rem; border-top: 1px solid var(--warm-border-color); + flex-wrap: wrap; + + `@media` (max-width: 600px) { + justify-content: center; + + > * { + flex: 1 1 100%; + } + } }src/app/modules/companies/components/company-review-block/company-review-block.component.scss (1)
116-166: Add focus-visible styles for interactive elements.
Hover-only cues can miss keyboard users; consider focus styles for.i-work-hereand.action-btn.♿ Suggested focus styles
.i-work-here { display: inline-flex; align-items: center; gap: 0.375rem; font-size: 0.8125rem; font-weight: 500; font-style: italic; color: var(--warm-accent-secondary); cursor: pointer; transition: color 0.2s ease; + &:focus-visible { + outline: 2px solid var(--warm-accent-primary); + outline-offset: 2px; + } } .review-actions { display: flex; gap: 0.75rem; .action-btn { display: inline-flex; align-items: center; gap: 0.375rem; padding: 0.375rem 0.75rem; border-radius: 6px; font-size: 0.8125rem; color: var(--warm-text-muted); background-color: transparent; border: 1px solid var(--warm-border-color); cursor: pointer; transition: all 0.2s ease; + &:focus-visible { + outline: 2px solid var(--warm-accent-primary); + outline-offset: 2px; + }src/app/modules/companies/components/recent-reviews/recent-reviews-page.component.scss (1)
34-124: Review header/footer may need wrapping on small screens.
Consider flex-wrap and a small-screen stack to avoid overflow.♻️ Suggested responsive tweak
.review-list-item { transition: all 0.25s ease; border-radius: 12px; padding: 1.25rem 1.5rem; border: 1px solid var(--warm-border-color); background-color: var(--warm-card-bg); color: var(--warm-text-primary); box-shadow: var(--warm-shadow-sm); .review-header { display: flex; justify-content: space-between; align-items: flex-start; margin-bottom: 0.75rem; + flex-wrap: wrap; + gap: 0.5rem 1rem; } .review-footer { display: flex; justify-content: space-between; align-items: center; margin-top: 1rem; padding-top: 0.75rem; border-top: 1px solid var(--warm-border-color); + flex-wrap: wrap; + gap: 0.5rem 1rem; } + `@media` (max-width: 600px) { + .review-footer { + flex-direction: column; + align-items: flex-start; + } + } }src/app/modules/companies/components/company-page/company-page.component.scss (1)
89-104: Section header may need responsive wrapping.
If the title and button crowd on small screens, add wrap/stack behavior.♻️ Suggested responsive tweak
.reviews-section { margin-top: 2rem; .section-header { display: flex; justify-content: space-between; align-items: center; margin-bottom: 1.5rem; + flex-wrap: wrap; + gap: 0.75rem 1rem; + + `@media` (max-width: 600px) { + flex-direction: column; + align-items: flex-start; + } } }src/app/modules/interviews/components/interviews-home/interviews-home.component.scss (1)
30-54: Consider reduced-motion fallback for hover transitions.
Respectprefers-reduced-motionfor image/card hover effects.♿ Suggested reduced-motion fallback
+@media (prefers-reduced-motion: reduce) { + .feature-image, + .feature-card { + transition: none; + transform: none; + } +}Also applies to: 78-121
src/app/modules/home/components/me/me.component.scss (1)
127-138: Consider replacing::ng-deepwith a safer styling hook.
::ng-deepis deprecated; it’s better to move the badge styling intoapp-user-roles-label(or expose a host class/CSS variable) to keep encapsulation intact.As per coding guidelines, consider avoiding deprecated
::ng-deep.src/app/modules/home/components/telegram-bot/telegram-bot.component.ts (1)
30-40: Consider adding explicit subscription cleanup for consistency.The
route.fragmentsubscription isn't explicitly unsubscribed. WhileActivatedRouteobservables are typically auto-cleaned by Angular's router, adding explicit cleanup (e.g., usinguntilDestroyed) would align with the subscription management pattern used elsewhere in this codebase (e.g., inNavbarComponent).♻️ Suggested improvement
+import { untilDestroyed } from "@shared/subscriptions/until-destroyed"; ... ngOnInit(): void { // Handle fragment scrolling for anchor links - this.route.fragment.subscribe((fragment) => { + this.route.fragment.pipe(untilDestroyed(this)).subscribe((fragment) => { if (fragment) { // Small delay to ensure the DOM is fully rendered setTimeout(() => { this.viewportScroller.scrollToAnchor(fragment); }, 100); } }); }src/app/components/navbar/navbar.component.ts (1)
55-69: Subscriptions lack explicit cleanup.The subscriptions to
loggedOutInvoked$,loggedOut$, andloggedIn$are not unsubscribed. WhilengOnDestroyis implemented, it's empty. Consider usinguntilDestroyed(this)for consistent subscription management.♻️ Suggested improvement
private setupSubscribers(): void { - this.authService.loggedOutInvoked$.subscribe(() => { + this.authService.loggedOutInvoked$.pipe(untilDestroyed(this)).subscribe(() => { this.currentUser = null; this.renderNavbar(); }); - this.authService.loggedOut$.subscribe(() => { + this.authService.loggedOut$.pipe(untilDestroyed(this)).subscribe(() => { this.currentUser = null; this.renderNavbar(); }); - this.authService.loggedIn$.subscribe((user) => { + this.authService.loggedIn$.pipe(untilDestroyed(this)).subscribe((user) => { this.currentUser = user; this.renderNavbar(); }); }src/app/components/admin-navbar/admin-navbar.component.html (1)
29-29: Consider addingtrackByfor the inner*ngForloop.Adding a
trackByfunction helps Angular optimize DOM updates when the list changes.src/app/components/admin-navbar/admin-navbar.component.scss (1)
50-54: Consider using a CSS variable instead of hardcodedwhite.For full theme consistency (especially if supporting dark mode), replace the hardcoded
whitewith a CSS variable likevar(--warm-text-on-accent)or similar.♻️ Suggested improvement
&:active { background-color: var(--warm-accent-primary); - color: white; + color: var(--warm-text-on-accent, `#ffffff`); }src/app/modules/home/components/home/home.component.scss (1)
44-60: Minor redundancy in responsive breakpoints.The
height: 60vhis repeated in all three breakpoints (default, min-width: 768px, min-width: 992px) with onlymax-widthchanging. Consider consolidating:♻️ Suggested simplification
.website-image { height: 60vh; width: auto; max-width: 100%; object-fit: contain; object-position: center; `@media` (min-width: 768px) { - height: 60vh; max-width: 80%; } `@media` (min-width: 992px) { - height: 60vh; max-width: 70%; } }REDESIGN.md (2)
361-368: Add language specifier to code block.Per static analysis (MD040), the fenced code block at line 361 should have a language specified for proper syntax highlighting.
📝 Suggested fix
-``` +```text src/ ├── _warm-theme.scss # NEW: All theme variables ├── _colors.scss # Keep for backward compatibility, import warm-theme ├── styles.scss # Updated to use warm theme └── app/ └── ...</details> --- `1-764`: **Comprehensive design system documentation.** Excellent coverage of the redesign plan including color palette, typography, implementation phases, accessibility considerations (WCAG AA compliance noted), and rollback strategy. The phased approach with file lists is practical for tracking progress. Note: Static analysis flagged multiple tables without surrounding blank lines (MD058). These are cosmetic markdown linting issues that don't affect rendering in most viewers, so addressing them is optional. </blockquote></details> <details> <summary>src/app/shared/services/theme/theme.service.ts (2)</summary><blockquote> `178-188`: **Consider extracting hardcoded theme colors to constants.** The meta theme-color values (`#1a1816`, `#fffbf7`) are hardcoded here and in the SCSS files. Consider extracting these to a shared constants file for single-source-of-truth maintenance, especially if the color palette evolves. --- `255-275`: **Hardcoded Russian strings are consistent with current application scope, but consider i18n if multi-language support is planned.** The theme labels in this service use hardcoded Russian strings. This is consistent with the existing codebase (e.g., `kazakhstan-city.ts` uses similar hardcoding). Since the project currently has no i18n framework configured and no multi-language support is evident, this is not a concern for the current scope. However, if internationalization becomes a future requirement, these strings should be externalized using Angular's i18n framework or a translation service. </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| ``` | ||
| src/app/ | ||
| ├── components/ # Global components (navbar, admin-navbar, maintenance) | ||
| ├── models/ # TypeScript interfaces and enums | ||
| ├── modules/ # Feature modules (lazy-loaded) | ||
| │ ├── admin/ # Admin panel | ||
| │ ├── companies/ # Company reviews | ||
| │ ├── home/ # Landing, auth callbacks, about pages | ||
| │ ├── interviews/ # Interview templates and sessions | ||
| │ ├── salaries/ # Salary charts and surveys | ||
| │ └── users/ # User profiles | ||
| ├── services/ # API clients (exported via index.ts barrel) | ||
| └── shared/ # Reusable components, directives, guards, interceptors | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "CLAUDE.md" -type fRepository: Techinterview-space/frontend
Length of output: 200
🏁 Script executed:
cat -n CLAUDE.md | sed -n '25,50p'Repository: Techinterview-space/frontend
Length of output: 1134
🏁 Script executed:
which markdownlint || npm list -g markdownlint || echo "Checking if markdownlint is available"Repository: Techinterview-space/frontend
Length of output: 305
🏁 Script executed:
markdownlint CLAUDE.md 2>&1 || echo "markdownlint may not be installed, checking for alternative tools"Repository: Techinterview-space/frontend
Length of output: 188
🏁 Script executed:
npm install -g markdownlint-cli2 2>&1 && markdownlint-cli2 CLAUDE.md 2>&1 || npx markdownlint-cli2 CLAUDE.md 2>&1Repository: Techinterview-space/frontend
Length of output: 4897
🏁 Script executed:
cd /usr/local/lib/node_modules && ls -la | grep markdownlintRepository: Techinterview-space/frontend
Length of output: 202
🏁 Script executed:
node -e "const ml = require('markdownlint'); const fs = require('fs'); const content = fs.readFileSync('CLAUDE.md', 'utf8'); const result = ml.sync({files: ['CLAUDE.md']}); console.log(JSON.stringify(result, null, 2));" 2>&1Repository: Techinterview-space/frontend
Length of output: 911
Add a language to the directory tree fence.
Markdownlint (MD040) requires a language on fenced code blocks; the directory tree starting at line 31 is missing one. Add text to the opening fence.
🛠️ Suggested fix
-```
+```text
src/app/
├── components/ # Global components (navbar, admin-navbar, maintenance)
├── models/ # TypeScript interfaces and enums
├── modules/ # Feature modules (lazy-loaded)
│ ├── admin/ # Admin panel
│ ├── companies/ # Company reviews
│ ├── home/ # Landing, auth callbacks, about pages
│ ├── interviews/ # Interview templates and sessions
│ ├── salaries/ # Salary charts and surveys
│ └── users/ # User profiles
├── services/ # API clients (exported via index.ts barrel)
└── shared/ # Reusable components, directives, guards, interceptors
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@CLAUDE.md` around lines 31 - 44, The fenced code block containing the
directory tree in CLAUDE.md is missing a language tag which triggers
markdownlint MD040; edit the opening fence for that block and add the language
identifier text (i.e., change the triple backtick fence that begins the
directory tree to ```text) so the block is properly annotated. Ensure you only
update the opening fence and leave the directory tree contents unchanged.
| // Shadows - Warm-tinted for cohesion | ||
| --warm-shadow-color: rgba(61, 45, 35, 0.08); | ||
| --warm-shadow-color-medium: rgba(61, 45, 35, 0.12); | ||
| --warm-shadow-color-strong: rgba(61, 45, 35, 0.16); | ||
|
|
||
| // Component-specific | ||
| --warm-input-bg: #ffffff; | ||
| --warm-input-border: rgba(61, 45, 35, 0.2); | ||
| --warm-input-focus-border: #e07a5f; | ||
| --warm-input-focus-ring: rgba(224, 122, 95, 0.25); | ||
|
|
||
| --warm-card-bg: #ffffff; | ||
| --warm-card-border: rgba(61, 45, 35, 0.08); | ||
|
|
||
| --warm-modal-overlay: rgba(45, 45, 45, 0.5); | ||
| --warm-modal-bg: #ffffff; | ||
|
|
||
| --warm-navbar-bg: #fffbf7; | ||
| --warm-navbar-border: rgba(61, 45, 35, 0.1); | ||
|
|
||
| --warm-footer-bg: #3d3d3d; | ||
| --warm-footer-text: #f5f5f5; | ||
| --warm-footer-muted: #c0c0c0; | ||
|
|
||
| // Chart colors | ||
| --warm-chart-1: #e07a5f; | ||
| --warm-chart-2: #81b29a; | ||
| --warm-chart-3: #f2cc8f; | ||
| --warm-chart-4: #f4a261; | ||
| --warm-chart-5: #6fa8c7; | ||
| --warm-chart-6: #c4a1d4; | ||
| --warm-chart-grid: rgba(61, 45, 35, 0.08); | ||
| --warm-chart-text: #5c5c5c; | ||
|
|
||
| // Code/monospace | ||
| --warm-code-bg: #fef7f0; | ||
| --warm-code-text: #3d3d3d; | ||
| --warm-code-border: rgba(61, 45, 35, 0.12); | ||
|
|
||
| // Table | ||
| --warm-table-stripe: rgba(61, 45, 35, 0.03); | ||
| --warm-table-hover: rgba(61, 45, 35, 0.06); | ||
| --warm-table-border: rgba(61, 45, 35, 0.1); | ||
|
|
||
| // Scrollbar | ||
| --warm-scrollbar-track: #fef7f0; | ||
| --warm-scrollbar-thumb: rgba(61, 45, 35, 0.2); | ||
| --warm-scrollbar-thumb-hover: rgba(61, 45, 35, 0.35); | ||
|
|
||
| // Transitions | ||
| --theme-transition: | ||
| background-color 0.3s ease, color 0.3s ease, border-color 0.3s ease, | ||
| box-shadow 0.3s ease, fill 0.3s ease, stroke 0.3s ease; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for shadow size variable usage
rg -n -- '--warm-shadow-(sm|md|lg|xl)\b' srcRepository: Techinterview-space/frontend
Length of output: 200
🏁 Script executed:
#!/bin/bash
# Check if shadow size vars are used in components
rg -n -- '--warm-shadow-(sm|md|lg|xl)\b' srcRepository: Techinterview-space/frontend
Length of output: 6648
🏁 Script executed:
#!/bin/bash
# Search for where these shadow vars are DEFINED (with --warm-shadow-* pattern)
rg -n -- '--warm-shadow-\w+\s*:' srcRepository: Techinterview-space/frontend
Length of output: 282
🏁 Script executed:
#!/bin/bash
# Look for global style files and check their contents
find src -name "*.scss" -o -name "*.css" | grep -E "(global|styles|theme)" | head -20Repository: Techinterview-space/frontend
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Check styles.scss for shadow variable definitions
grep -n -- '--warm-shadow' src/styles.scss
# Also check if it imports _warm-theme.scss
grep -n '@import' src/styles.scss | head -10Repository: Techinterview-space/frontend
Length of output: 313
🏁 Script executed:
#!/bin/bash
# Check context around that shadow usage in styles.scss (lines 1248-1249)
sed -n '1240,1260p' src/styles.scssRepository: Techinterview-space/frontend
Length of output: 787
🏁 Script executed:
#!/bin/bash
# Check the full _warm-theme.scss file to see all shadow definitions
cat -n src/_warm-theme.scss | head -150Repository: Techinterview-space/frontend
Length of output: 6209
🏁 Script executed:
#!/bin/bash
# Check the rest of the file for shadow-size variables
tail -n +150 src/_warm-theme.scss | head -150Repository: Techinterview-space/frontend
Length of output: 4548
Define missing shadow-size CSS variables used by components.
Components across the application reference var(--warm-shadow-sm), var(--warm-shadow-md), var(--warm-shadow-lg), and var(--warm-shadow-xl), but only shadow color variables are defined in this file. These shadow-size variables are completely missing, causing shadows to fail rendering across 40+ component files.
Proposed fix
// Shadows - Warm-tinted for cohesion
--warm-shadow-color: rgba(61, 45, 35, 0.08);
--warm-shadow-color-medium: rgba(61, 45, 35, 0.12);
--warm-shadow-color-strong: rgba(61, 45, 35, 0.16);
+ --warm-shadow-sm:
+ 0 1px 2px var(--warm-shadow-color),
+ 0 2px 4px var(--warm-shadow-color);
+ --warm-shadow-md:
+ 0 2px 4px var(--warm-shadow-color),
+ 0 4px 8px var(--warm-shadow-color),
+ 0 8px 16px var(--warm-shadow-color);
+ --warm-shadow-lg:
+ 0 4px 8px var(--warm-shadow-color),
+ 0 8px 16px var(--warm-shadow-color),
+ 0 16px 32px var(--warm-shadow-color-medium);
+ --warm-shadow-xl:
+ 0 8px 16px var(--warm-shadow-color),
+ 0 16px 32px var(--warm-shadow-color-medium),
+ 0 32px 64px var(--warm-shadow-color-strong);📝 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.
| // Shadows - Warm-tinted for cohesion | |
| --warm-shadow-color: rgba(61, 45, 35, 0.08); | |
| --warm-shadow-color-medium: rgba(61, 45, 35, 0.12); | |
| --warm-shadow-color-strong: rgba(61, 45, 35, 0.16); | |
| // Component-specific | |
| --warm-input-bg: #ffffff; | |
| --warm-input-border: rgba(61, 45, 35, 0.2); | |
| --warm-input-focus-border: #e07a5f; | |
| --warm-input-focus-ring: rgba(224, 122, 95, 0.25); | |
| --warm-card-bg: #ffffff; | |
| --warm-card-border: rgba(61, 45, 35, 0.08); | |
| --warm-modal-overlay: rgba(45, 45, 45, 0.5); | |
| --warm-modal-bg: #ffffff; | |
| --warm-navbar-bg: #fffbf7; | |
| --warm-navbar-border: rgba(61, 45, 35, 0.1); | |
| --warm-footer-bg: #3d3d3d; | |
| --warm-footer-text: #f5f5f5; | |
| --warm-footer-muted: #c0c0c0; | |
| // Chart colors | |
| --warm-chart-1: #e07a5f; | |
| --warm-chart-2: #81b29a; | |
| --warm-chart-3: #f2cc8f; | |
| --warm-chart-4: #f4a261; | |
| --warm-chart-5: #6fa8c7; | |
| --warm-chart-6: #c4a1d4; | |
| --warm-chart-grid: rgba(61, 45, 35, 0.08); | |
| --warm-chart-text: #5c5c5c; | |
| // Code/monospace | |
| --warm-code-bg: #fef7f0; | |
| --warm-code-text: #3d3d3d; | |
| --warm-code-border: rgba(61, 45, 35, 0.12); | |
| // Table | |
| --warm-table-stripe: rgba(61, 45, 35, 0.03); | |
| --warm-table-hover: rgba(61, 45, 35, 0.06); | |
| --warm-table-border: rgba(61, 45, 35, 0.1); | |
| // Scrollbar | |
| --warm-scrollbar-track: #fef7f0; | |
| --warm-scrollbar-thumb: rgba(61, 45, 35, 0.2); | |
| --warm-scrollbar-thumb-hover: rgba(61, 45, 35, 0.35); | |
| // Transitions | |
| --theme-transition: | |
| background-color 0.3s ease, color 0.3s ease, border-color 0.3s ease, | |
| box-shadow 0.3s ease, fill 0.3s ease, stroke 0.3s ease; | |
| } | |
| // Shadows - Warm-tinted for cohesion | |
| --warm-shadow-color: rgba(61, 45, 35, 0.08); | |
| --warm-shadow-color-medium: rgba(61, 45, 35, 0.12); | |
| --warm-shadow-color-strong: rgba(61, 45, 35, 0.16); | |
| --warm-shadow-sm: | |
| 0 1px 2px var(--warm-shadow-color), | |
| 0 2px 4px var(--warm-shadow-color); | |
| --warm-shadow-md: | |
| 0 2px 4px var(--warm-shadow-color), | |
| 0 4px 8px var(--warm-shadow-color), | |
| 0 8px 16px var(--warm-shadow-color); | |
| --warm-shadow-lg: | |
| 0 4px 8px var(--warm-shadow-color), | |
| 0 8px 16px var(--warm-shadow-color), | |
| 0 16px 32px var(--warm-shadow-color-medium); | |
| --warm-shadow-xl: | |
| 0 8px 16px var(--warm-shadow-color), | |
| 0 16px 32px var(--warm-shadow-color-medium), | |
| 0 32px 64px var(--warm-shadow-color-strong); | |
| // Component-specific | |
| --warm-input-bg: `#ffffff`; | |
| --warm-input-border: rgba(61, 45, 35, 0.2); | |
| --warm-input-focus-border: `#e07a5f`; | |
| --warm-input-focus-ring: rgba(224, 122, 95, 0.25); | |
| --warm-card-bg: `#ffffff`; | |
| --warm-card-border: rgba(61, 45, 35, 0.08); | |
| --warm-modal-overlay: rgba(45, 45, 45, 0.5); | |
| --warm-modal-bg: `#ffffff`; | |
| --warm-navbar-bg: `#fffbf7`; | |
| --warm-navbar-border: rgba(61, 45, 35, 0.1); | |
| --warm-footer-bg: `#3d3d3d`; | |
| --warm-footer-text: `#f5f5f5`; | |
| --warm-footer-muted: `#c0c0c0`; | |
| // Chart colors | |
| --warm-chart-1: `#e07a5f`; | |
| --warm-chart-2: `#81b29a`; | |
| --warm-chart-3: `#f2cc8f`; | |
| --warm-chart-4: `#f4a261`; | |
| --warm-chart-5: `#6fa8c7`; | |
| --warm-chart-6: `#c4a1d4`; | |
| --warm-chart-grid: rgba(61, 45, 35, 0.08); | |
| --warm-chart-text: `#5c5c5c`; | |
| // Code/monospace | |
| --warm-code-bg: `#fef7f0`; | |
| --warm-code-text: `#3d3d3d`; | |
| --warm-code-border: rgba(61, 45, 35, 0.12); | |
| // Table | |
| --warm-table-stripe: rgba(61, 45, 35, 0.03); | |
| --warm-table-hover: rgba(61, 45, 35, 0.06); | |
| --warm-table-border: rgba(61, 45, 35, 0.1); | |
| // Scrollbar | |
| --warm-scrollbar-track: `#fef7f0`; | |
| --warm-scrollbar-thumb: rgba(61, 45, 35, 0.2); | |
| --warm-scrollbar-thumb-hover: rgba(61, 45, 35, 0.35); | |
| // Transitions | |
| --theme-transition: | |
| background-color 0.3s ease, color 0.3s ease, border-color 0.3s ease, | |
| box-shadow 0.3s ease, fill 0.3s ease, stroke 0.3s ease; | |
| } |
🤖 Prompt for AI Agents
In `@src/_warm-theme.scss` around lines 57 - 110, Define the missing shadow-size
CSS variables (--warm-shadow-sm, --warm-shadow-md, --warm-shadow-lg,
--warm-shadow-xl) in the same :root / theme block so components using
var(--warm-shadow-*) render; create them as box-shadow strings that reference
the existing color variables (--warm-shadow-color, --warm-shadow-color-medium,
--warm-shadow-color-strong) and appropriate offsets/blur (e.g., small: subtle 0
1px 2px, medium: 0 4px 8px, large: 0 10px 20px, xl: 0 20px 40px) so components
like those expecting --warm-shadow-sm / --warm-shadow-md / --warm-shadow-lg /
--warm-shadow-xl receive concrete box-shadow values.
| // Component-specific styles for OpenAI Prompts admin page | ||
| .text-truncate { | ||
| overflow: hidden; | ||
| text-overflow: ellipsis; | ||
| white-space: nowrap; | ||
| max-width: 300px; | ||
| } |
There was a problem hiding this comment.
Inconsistency between SCSS and HTML inline style.
The .text-truncate class defines max-width: 300px here, but the corresponding HTML template uses an inline style with max-width: 210px. The inline style will override this rule, making the SCSS definition ineffective.
Consider removing the inline style from the HTML and updating this value to 210px, or using a modifier class for different widths.
Suggested fix
.text-truncate {
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
- max-width: 300px;
+ max-width: 210px;
}
Then remove the inline style from the HTML template.
📝 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.
| // Component-specific styles for OpenAI Prompts admin page | |
| .text-truncate { | |
| overflow: hidden; | |
| text-overflow: ellipsis; | |
| white-space: nowrap; | |
| max-width: 300px; | |
| } | |
| // Component-specific styles for OpenAI Prompts admin page | |
| .text-truncate { | |
| overflow: hidden; | |
| text-overflow: ellipsis; | |
| white-space: nowrap; | |
| max-width: 210px; | |
| } |
🤖 Prompt for AI Agents
In
`@src/app/modules/admin/components/openai-prompts/openai-prompts-admin-page.component.scss`
around lines 31 - 37, The .text-truncate SCSS max-width (in OpenAI Prompts admin
styles) conflicts with an inline style in the component template; update the
styling to be consistent by either changing .text-truncate's max-width to 210px
and removing the inline style from the OpenaiPromptsAdminPage component
template, or introduce a modifier class (e.g., .text-truncate--sm { max-width:
210px }) and replace the inline style with that class in the template so the
CSS, not inline styles, controls the width.
| <div class="container mt-5 mb-5"> | ||
| <app-data-loader [title]="''"> | ||
| <div class="mt-3"> | ||
| <div class="text-muted h2 d-flex justify-content-center"> | ||
| Загружаем статистику | ||
| </div> | ||
| <div class="mt-3 d-flex justify-content-center"> | ||
| <img | ||
| src="https://techinterview.fra1.cdn.digitaloceanspaces.com/images/main_charts_transparent_1000.png" | ||
| class="img-fluid mobile-w-30" | ||
| alt="logo" | ||
| title="Кот Магеллан, который любит считать деньги" | ||
| /> | ||
| </div> | ||
| </div> | ||
| </app-data-loader> | ||
| <app-data-loader [title]="''" /> | ||
| </div> |
There was a problem hiding this comment.
Provide accessible loading text.
Line 525 uses an empty title, so users (and screen readers) lose context in the loading state. Consider a localized loading string or an aria-label.
✅ Suggested tweak
- <app-data-loader [title]="''" />
+ <app-data-loader [title]="'Загрузка...'" />
📝 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.
| <div class="container mt-5 mb-5"> | |
| <app-data-loader [title]="''"> | |
| <div class="mt-3"> | |
| <div class="text-muted h2 d-flex justify-content-center"> | |
| Загружаем статистику | |
| </div> | |
| <div class="mt-3 d-flex justify-content-center"> | |
| <img | |
| src="https://techinterview.fra1.cdn.digitaloceanspaces.com/images/main_charts_transparent_1000.png" | |
| class="img-fluid mobile-w-30" | |
| alt="logo" | |
| title="Кот Магеллан, который любит считать деньги" | |
| /> | |
| </div> | |
| </div> | |
| </app-data-loader> | |
| <app-data-loader [title]="''" /> | |
| </div> | |
| <div class="container mt-5 mb-5"> | |
| <app-data-loader [title]="'Загрузка...'" /> | |
| </div> |
🤖 Prompt for AI Agents
In
`@src/app/modules/salaries/components/salaries-chart/salaries-chart.component.html`
around lines 524 - 526, The app-data-loader is rendered with an empty title
which removes context for screen readers; update the usage in
salaries-chart.component.html to pass a meaningful, localized loading string
into the [title] input (or add an aria-label) instead of an empty string — e.g.,
provide a localized key like "salaries.loading" or call the component's
i18n/gettext helper and bind it to [title] (or add aria-label="...") so screen
readers receive descriptive loading text when app-data-loader is visible.
| // Dialog component - uses global .modal styles from styles.scss | ||
| // No component-specific overrides needed |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and check the dialog.component.scss file
find . -name "dialog.component.scss" -type f 2>/dev/null | head -5Repository: Techinterview-space/frontend
Length of output: 136
🏁 Script executed:
#!/bin/bash
# Search for .modal styles in styles.scss
find . -name "styles.scss" -type f 2>/dev/null | head -5Repository: Techinterview-space/frontend
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Search for global modal styles
rg -n "\.modal\s*\{" --type=scss -A 15 2>/dev/nullRepository: Techinterview-space/frontend
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Read the dialog component styles file
cat -n ./src/app/shared/components/dialogs/dialog/dialog.component.scssRepository: Techinterview-space/frontend
Length of output: 192
🏁 Script executed:
#!/bin/bash
# Read the global styles file
wc -l ./src/styles.scssRepository: Techinterview-space/frontend
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Search for modal and dialog related styles in styles.scss
rg -n "(modal|dialog)" --type=scss -i -B 2 -A 5 ./src/styles.scssRepository: Techinterview-space/frontend
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Search for modal and dialog styles in styles.scss without type restriction
rg -n "(modal|dialog)" -i -B 2 -A 5 ./src/styles.scss 2>/dev/null | head -100Repository: Techinterview-space/frontend
Length of output: 926
🏁 Script executed:
#!/bin/bash
# Alternative: search for CSS class patterns that might be modal-related
rg -n "^\." ./src/styles.scss | grep -i "modal\|dialog" | head -20Repository: Techinterview-space/frontend
Length of output: 145
🏁 Script executed:
#!/bin/bash
# Check if there's any modal styling at all in styles.scss
cat ./src/styles.scss | grep -i "modal" -n -A 5 | head -50Repository: Techinterview-space/frontend
Length of output: 830
🏁 Script executed:
#!/bin/bash
# Search for CSS variable definitions for dark mode
rg -n "warm-modal|dark-modal" ./src/styles.scss -B 1 -A 1 | head -30Repository: Techinterview-space/frontend
Length of output: 293
🏁 Script executed:
#!/bin/bash
# Find the dialog component template
find ./src -name "dialog.component.html" -type f 2>/dev/nullRepository: Techinterview-space/frontend
Length of output: 136
🏁 Script executed:
#!/bin/bash
# Check if modal styles include responsive design patterns
rg -n "@media|responsive" ./src/styles.scss -B 2 -A 2 | head -50Repository: Techinterview-space/frontend
Length of output: 602
🏁 Script executed:
#!/bin/bash
# Search for dark mode/theme CSS variables
rg -n "dark-modal|--dark|--night|color-scheme" ./src/styles.scss -B 1 -A 1 | head -50Repository: Techinterview-space/frontend
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check dialog component template content
cat -n ./src/app/shared/components/dialogs/dialog/dialog.component.htmlRepository: Techinterview-space/frontend
Length of output: 1050
🏁 Script executed:
#!/bin/bash
# Check for CSS variables definitions in the file
rg -n "^\s*--" ./src/styles.scss | head -30Repository: Techinterview-space/frontend
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Search for CSS custom property definitions and theme handling
rg -n ":root|@media.*dark|color-scheme|prefers-color-scheme" ./src/styles.scss -B 1 -A 5 | head -80Repository: Techinterview-space/frontend
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check if there's a separate theme or dark mode file
find ./src -name "*theme*" -o -name "*dark*" -type f 2>/dev/null | grep -E "\.(scss|css|ts)$"Repository: Techinterview-space/frontend
Length of output: 143
🏁 Script executed:
#!/bin/bash
# Check if modal has specific mobile/responsive styling needs by looking at dialog component
grep -n "mobile\|responsive\|tablet\|max-width" ./src/app/shared/components/dialogs/dialog/dialog.component.htmlRepository: Techinterview-space/frontend
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check the warm theme file for modal variable definitions
cat ./src/_warm-theme.scss | grep -n "modal" -i -B 1 -A 2 | head -50Repository: Techinterview-space/frontend
Length of output: 450
🏁 Script executed:
#!/bin/bash
# Read the theme service to understand theming approach
head -100 ./src/app/shared/services/theme/theme.service.tsRepository: Techinterview-space/frontend
Length of output: 2461
🏁 Script executed:
#!/bin/bash
# Check if there are responsive design patterns for modal in styles.scss
rg -n "\.modal" ./src/styles.scss -A 20 | grep "@media" -B 5 -A 5Repository: Techinterview-space/frontend
Length of output: 54
Verify responsive design patterns in global modal styles.
Removing component-scoped styles in favor of global .modal styles is valid. Global styles in styles.scss properly define modal structure (background, animations, borders) with CSS variables for theming. Both light and dark mode variants are correctly defined in _warm-theme.scss. However, the modal styles lack responsive design patterns (@media queries) for mobile optimization—consider adding mobile-specific adjustments (e.g., width, padding, max-height for smaller screens).
🤖 Prompt for AI Agents
In `@src/app/shared/components/dialogs/dialog/dialog.component.scss` around lines
1 - 2, The global modal styles lack mobile-responsive rules—update the global
styles (where .modal is defined in styles.scss and theme tokens in
_warm-theme.scss) to add `@media` queries for small viewports that reduce modal
width/max-width, decrease padding, set a sensible max-height with overflow:auto
or -webkit-overflow-scrolling:touch, and ensure border-radius and animations
still work with CSS variables; adjust any theme variable fallbacks in
_warm-theme.scss if needed and remove or leave dialog.component.scss empty since
component-specific overrides aren’t required.
| private initSystemPreferenceDetection(): void { | ||
| this.mediaQuery = window.matchMedia("(prefers-color-scheme: dark)"); | ||
| this._systemPrefersDark.set(this.mediaQuery.matches); | ||
|
|
||
| // Listen for system preference changes | ||
| const handler = (event: MediaQueryListEvent) => { | ||
| this._systemPrefersDark.set(event.matches); | ||
| }; | ||
|
|
||
| // Modern browsers | ||
| if (this.mediaQuery.addEventListener) { | ||
| this.mediaQuery.addEventListener("change", handler); | ||
| } else { | ||
| // Fallback for older browsers | ||
| this.mediaQuery.addListener(handler); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing cleanup for MediaQueryList listener.
The event listener added to mediaQuery is never removed. While this service is providedIn: 'root' (singleton) and unlikely to be destroyed during normal app lifecycle, the listener should be cleaned up for proper resource management, especially in testing scenarios.
🔧 Suggested fix with cleanup
+import { OnDestroy } from "@angular/core";
`@Injectable`({
providedIn: "root",
})
-export class ThemeService {
+export class ThemeService implements OnDestroy {
private readonly isBrowser: boolean;
+ private mediaQueryHandler: ((event: MediaQueryListEvent) => void) | null = null;
// ... existing code ...
private initSystemPreferenceDetection(): void {
this.mediaQuery = window.matchMedia("(prefers-color-scheme: dark)");
this._systemPrefersDark.set(this.mediaQuery.matches);
- const handler = (event: MediaQueryListEvent) => {
+ this.mediaQueryHandler = (event: MediaQueryListEvent) => {
this._systemPrefersDark.set(event.matches);
};
if (this.mediaQuery.addEventListener) {
- this.mediaQuery.addEventListener("change", handler);
+ this.mediaQuery.addEventListener("change", this.mediaQueryHandler);
} else {
- this.mediaQuery.addListener(handler);
+ this.mediaQuery.addListener(this.mediaQueryHandler);
}
}
+ ngOnDestroy(): void {
+ if (this.mediaQuery && this.mediaQueryHandler) {
+ if (this.mediaQuery.removeEventListener) {
+ this.mediaQuery.removeEventListener("change", this.mediaQueryHandler);
+ } else {
+ this.mediaQuery.removeListener(this.mediaQueryHandler);
+ }
+ }
+ }
📝 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.
| private initSystemPreferenceDetection(): void { | |
| this.mediaQuery = window.matchMedia("(prefers-color-scheme: dark)"); | |
| this._systemPrefersDark.set(this.mediaQuery.matches); | |
| // Listen for system preference changes | |
| const handler = (event: MediaQueryListEvent) => { | |
| this._systemPrefersDark.set(event.matches); | |
| }; | |
| // Modern browsers | |
| if (this.mediaQuery.addEventListener) { | |
| this.mediaQuery.addEventListener("change", handler); | |
| } else { | |
| // Fallback for older browsers | |
| this.mediaQuery.addListener(handler); | |
| } | |
| } | |
| import { OnDestroy } from "@angular/core"; | |
| `@Injectable`({ | |
| providedIn: "root", | |
| }) | |
| export class ThemeService implements OnDestroy { | |
| private readonly isBrowser: boolean; | |
| private mediaQueryHandler: ((event: MediaQueryListEvent) => void) | null = null; | |
| // ... existing code ... | |
| private initSystemPreferenceDetection(): void { | |
| this.mediaQuery = window.matchMedia("(prefers-color-scheme: dark)"); | |
| this._systemPrefersDark.set(this.mediaQuery.matches); | |
| // Listen for system preference changes | |
| this.mediaQueryHandler = (event: MediaQueryListEvent) => { | |
| this._systemPrefersDark.set(event.matches); | |
| }; | |
| // Modern browsers | |
| if (this.mediaQuery.addEventListener) { | |
| this.mediaQuery.addEventListener("change", this.mediaQueryHandler); | |
| } else { | |
| // Fallback for older browsers | |
| this.mediaQuery.addListener(this.mediaQueryHandler); | |
| } | |
| } | |
| ngOnDestroy(): void { | |
| if (this.mediaQuery && this.mediaQueryHandler) { | |
| if (this.mediaQuery.removeEventListener) { | |
| this.mediaQuery.removeEventListener("change", this.mediaQueryHandler); | |
| } else { | |
| this.mediaQuery.removeListener(this.mediaQueryHandler); | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/app/shared/services/theme/theme.service.ts` around lines 124 - 140, The
initSystemPreferenceDetection method adds a MediaQueryList listener
(this.mediaQuery) via addEventListener/addListener but never removes it; update
the service to store the handler (e.g., a private property like this._mqHandler
or reuse the existing handler reference) and implement cleanup by removing the
listener on destroy: implement OnDestroy on the service and in ngOnDestroy call
this.mediaQuery.removeEventListener("change", handler) with a fallback to
this.mediaQuery.removeListener(handler) for older browsers, ensuring the same
handler reference is used so removal succeeds.
| <!-- Theme initialization script - runs before body to prevent flash --> | ||
| <script> | ||
| (function () { | ||
| // Check for saved theme preference or system preference | ||
| const savedTheme = localStorage.getItem("theme"); | ||
| const systemPrefersDark = window.matchMedia( | ||
| "(prefers-color-scheme: dark)", | ||
| ).matches; | ||
|
|
||
| // Apply theme to root element | ||
| if ( | ||
| savedTheme === "dark" || | ||
| (savedTheme === null && systemPrefersDark) | ||
| ) { | ||
| document.documentElement.setAttribute("data-theme", "dark"); | ||
| } else if (savedTheme === "light") { | ||
| document.documentElement.setAttribute("data-theme", "light"); | ||
| } | ||
| // If no saved theme and system is light, don't set attribute (default light) | ||
|
|
||
| // Remove no-transitions class after page load to enable smooth transitions | ||
| window.addEventListener("load", function () { | ||
| setTimeout(function () { | ||
| document.documentElement.classList.remove("no-transitions"); | ||
| }, 100); | ||
| }); | ||
| })(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for ThemeService and theme-related code
rg -nP 'class ThemeService|export.*ThemeService|function.*ThemeService' --type ts --type js
Repository: Techinterview-space/frontend
Length of output: 149
🏁 Script executed:
# Search for all localStorage theme access patterns
rg -nP 'localStorage\.(getItem|setItem|removeItem)\(\s*["\']theme["\']' --type ts --type js --type html -A 2 -B 2
Repository: Techinterview-space/frontend
Length of output: 140
🏁 Script executed:
# Search for all theme value assignments and data-theme attributes
rg -nP 'data-theme|setAttribute.*theme|"(dark|light|system)"' --type ts --type js --type html -B 1 -A 1
Repository: Techinterview-space/frontend
Length of output: 6137
🏁 Script executed:
# Check for existing try-catch patterns around localStorage in the codebase
rg -nP 'try\s*\{[\s\S]*?localStorage' --type ts --type js -B 2 -A 5
Repository: Techinterview-space/frontend
Length of output: 54
🏁 Script executed:
# Get the full ThemeService implementation
cat -n src/app/shared/services/theme/theme.service.ts | head -250
Repository: Techinterview-space/frontend
Length of output: 8259
🏁 Script executed:
# Look for where setTheme is called and how localStorage is managed
rg -nP 'setTheme|localStorage.setItem.*theme' src/app/shared/services/theme/theme.service.ts -B 3 -A 5
Repository: Techinterview-space/frontend
Length of output: 1566
🏁 Script executed:
# Check when ThemeService initializes relative to the inline script
rg -nP 'ThemeService|providers.*ThemeService' src/main.ts src/app.component.ts --type ts -B 2 -A 2
Repository: Techinterview-space/frontend
Length of output: 132
🏁 Script executed:
# Search for direct localStorage writes to ensure "system" is never written
rg -nP 'localStorage\.setItem.*system|localStorage\.setItem.*theme' --type ts --type js
Repository: Techinterview-space/frontend
Length of output: 178
🏁 Script executed:
# Check if there are other theme initialization points
rg -nP 'bootstrap|bootstrapModule|provideApplicationConfig' src/main.ts -B 2 -A 5
Repository: Techinterview-space/frontend
Length of output: 178
🏁 Script executed:
# Search for any app initialization or bootstrap configuration
cat -n src/main.ts | head -50
Repository: Techinterview-space/frontend
Length of output: 539
Add try/catch guard around localStorage access to handle restricted storage modes.
The localStorage.getItem call can throw in restricted storage contexts (private browsing, some browsers, or CSP restrictions). The concern about handling a "system" value is unnecessary—ThemeService never persists "system" to localStorage; it only stores "light" or "dark", or removes the entry entirely to represent system preference.
🛠️ Suggested fix
<script>
(function () {
// Check for saved theme preference or system preference
- const savedTheme = localStorage.getItem("theme");
+ let savedTheme = null;
+ try {
+ savedTheme = localStorage.getItem("theme");
+ } catch (e) {
+ // localStorage may be unavailable in restricted storage modes
+ }
const systemPrefersDark = window.matchMedia(
"(prefers-color-scheme: dark)",
).matches;
// Apply theme to root element
if (
savedTheme === "dark" ||
(savedTheme === null && systemPrefersDark)
) {
document.documentElement.setAttribute("data-theme", "dark");
} else if (savedTheme === "light") {
document.documentElement.setAttribute("data-theme", "light");
}
📝 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.
| <!-- Theme initialization script - runs before body to prevent flash --> | |
| <script> | |
| (function () { | |
| // Check for saved theme preference or system preference | |
| const savedTheme = localStorage.getItem("theme"); | |
| const systemPrefersDark = window.matchMedia( | |
| "(prefers-color-scheme: dark)", | |
| ).matches; | |
| // Apply theme to root element | |
| if ( | |
| savedTheme === "dark" || | |
| (savedTheme === null && systemPrefersDark) | |
| ) { | |
| document.documentElement.setAttribute("data-theme", "dark"); | |
| } else if (savedTheme === "light") { | |
| document.documentElement.setAttribute("data-theme", "light"); | |
| } | |
| // If no saved theme and system is light, don't set attribute (default light) | |
| // Remove no-transitions class after page load to enable smooth transitions | |
| window.addEventListener("load", function () { | |
| setTimeout(function () { | |
| document.documentElement.classList.remove("no-transitions"); | |
| }, 100); | |
| }); | |
| })(); | |
| <!-- Theme initialization script - runs before body to prevent flash --> | |
| <script> | |
| (function () { | |
| // Check for saved theme preference or system preference | |
| let savedTheme = null; | |
| try { | |
| savedTheme = localStorage.getItem("theme"); | |
| } catch (e) { | |
| // localStorage may be unavailable in restricted storage modes | |
| } | |
| const systemPrefersDark = window.matchMedia( | |
| "(prefers-color-scheme: dark)", | |
| ).matches; | |
| // Apply theme to root element | |
| if ( | |
| savedTheme === "dark" || | |
| (savedTheme === null && systemPrefersDark) | |
| ) { | |
| document.documentElement.setAttribute("data-theme", "dark"); | |
| } else if (savedTheme === "light") { | |
| document.documentElement.setAttribute("data-theme", "light"); | |
| } | |
| // If no saved theme and system is light, don't set attribute (default light) | |
| // Remove no-transitions class after page load to enable smooth transitions | |
| window.addEventListener("load", function () { | |
| setTimeout(function () { | |
| document.documentElement.classList.remove("no-transitions"); | |
| }, 100); | |
| }); | |
| })(); | |
| </script> |
🤖 Prompt for AI Agents
In `@src/index.html` around lines 73 - 99, Wrap the localStorage access in a
try/catch around the localStorage.getItem("theme") call so restricted storage
errors don’t throw; when an exception occurs, treat savedTheme as null (fall
back to systemPrefersDark detection) and proceed to set
document.documentElement.setAttribute("data-theme", "dark") only when savedTheme
=== "dark" or savedTheme is null and systemPrefersDark is true, otherwise set
"light" only when savedTheme === "light"; keep the window.load listener and
no-transitions removal unchanged.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Style
✏️ Tip: You can customize this high-level summary in your review settings.