Allow user to configure light/dark mode in setting; cache accepted terms - #87
Allow user to configure light/dark mode in setting; cache accepted terms#87nllong wants to merge 3 commits into
Conversation
…d terms for 90 days
| import { TermsService } from './terms.service' | ||
|
|
||
| const ACCEPTED_AT_KEY = 'nlrTermsAcceptedAt' | ||
| const ACCEPTANCE_DAYS = 90 |
There was a problem hiding this comment.
keep accepted terms for 90 days
There was a problem hiding this comment.
Pull request overview
Adds user-facing controls for display preferences and reduces repeated Terms-of-Service prompts by caching acceptance on the client. This fits into the profile/auth experience by persisting a user’s preferred light/dark scheme and streamlining sign-in when terms were recently accepted.
Changes:
- Added a new Profile “Display” page to choose light/dark mode and persist the selection to org-user settings.
- Hydrated the app-wide theme scheme from the signed-in user’s persisted setting at layout startup.
- Cached NLR Terms acceptance in localStorage (90-day window) and updated sign-in UX + added unit tests.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/modules/profile/settings/settings.component.ts | New component to persist a selected scheme to org-user settings and update ConfigService. |
| src/app/modules/profile/settings/settings.component.html | New UI for selecting light/dark via a button-toggle group. |
| src/app/modules/profile/settings/settings.component.spec.ts | Unit tests for persisting scheme changes and rollback behavior. |
| src/app/modules/profile/profile.routes.ts | Adds /profile/display route for the new settings page. |
| src/app/modules/profile/profile.component.ts | Adds “Display” to profile tabs/navigation. |
| src/app/layout/layout.component.ts | Sets initial scheme from currentUser.settings.colorScheme. |
| src/app/layout/common/user/user.component.ts | Adds navigation handler to open display settings. |
| src/app/layout/common/user/user.component.html | Wires the Settings menu item to navigate to /profile/display. |
| src/app/modules/auth/sign-in/sign-in.component.ts | Initializes terms checkbox based on cached acceptance and records acceptance on successful sign-in. |
| src/app/modules/auth/sign-in/sign-in.component.html | Shows an “already accepted” state and hides the checkbox when cached acceptance is valid. |
| src/@seed/services/terms/terms.service.ts | Adds localStorage-backed acceptance caching with a 90-day expiry. |
| src/@seed/services/terms/terms.service.spec.ts | Unit tests for acceptance recording and expiry logic. |
| src/@seed/api/organization/organization.types.ts | Extends org-user settings type with optional colorScheme. |
| MIGRATION.md | Updates migration snapshot/status narrative (appears unrelated to stated PR purpose). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| <mat-button-toggle-group | ||
| class="border-button-toggle-group flex w-full" | ||
| [disabled]="saving" | ||
| [value]="scheme" | ||
| aria-label="Color scheme" | ||
| > | ||
| <mat-button-toggle class="flex-1" (click)="setScheme('light')" value="light"> | ||
| <mat-icon class="mr-2 icon-size-5" svgIcon="heroicons-solid:sun"></mat-icon> | ||
| Light | ||
| </mat-button-toggle> | ||
| <mat-button-toggle class="flex-1" (click)="setScheme('dark')" value="dark"> | ||
| <mat-icon class="mr-2 icon-size-5" svgIcon="heroicons-solid:moon"></mat-icon> | ||
| Dark | ||
| </mat-button-toggle> | ||
| </mat-button-toggle-group> |
There was a problem hiding this comment.
Fixed in the latest commit — moved the scheme persistence from per-toggle (click) handlers to a single (change)="setScheme($event.value)" on the mat-button-toggle-group, so all input methods (keyboard, mouse, etc.) trigger the save correctly.
| <h2 class="mb-6 flex items-center text-center text-2xl"> | ||
| <mat-icon class="mr-2 text-primary-900 dark:text-primary-300" svgIcon="fa-solid:gear"></mat-icon> | ||
| Display | ||
| </h2> |
| setScheme(scheme: ColorScheme): void { | ||
| if (this.saving || this.currentUser.settings.colorScheme === scheme) return |
| const NLR_TERMS_ACCEPTED_AT_KEY = 'nlrTermsAcceptedAt' | ||
| const NLR_TERMS_ACCEPTANCE_DAYS = 90 | ||
| const MILLISECONDS_PER_DAY = 24 * 60 * 60 * 1000 |
There was a problem hiding this comment.
Fixed in the latest commit. hasAcceptedTerms and recordTermsAcceptance now both take an email: string parameter and scope the localStorage key to nlrTermsAcceptedAt:${email.toLowerCase().trim()}, so acceptance for one account can no longer satisfy the requirement for a different account. In sign-in.component.ts, the check is done reactively via email.valueChanges (so it evaluates only once a valid address is entered), and the email is passed when recording acceptance after successful sign-in.
Co-authored-by: nllong <1907354+nllong@users.noreply.github.com>
Co-authored-by: nllong <1907354+nllong@users.noreply.github.com>
No description provided.