Skip to content

fix: Persist Settings Panel preferences using Zustand store #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
BenGWeeks wants to merge 1 commit into from
Closed

fix: Persist Settings Panel preferences using Zustand store #23

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/components/settings/SettingsPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useState, useEffect } from 'react';
import { Card } from '@/components/ui';
import { bcClient } from '@/services/bc/bcClient';
import { useAuth } from '@/services/auth';
import { useSettingsStore } from '@/hooks/useSettingsStore';
import {
BuildingOffice2Icon,
EnvelopeIcon,
Expand All @@ -26,6 +27,7 @@ interface CompanyInfo {

export function SettingsPanel() {
const { account } = useAuth();
const { weeklyHoursTarget, notificationsEnabled, updateSettings } = useSettingsStore();
const [companyInfo, setCompanyInfo] = useState<CompanyInfo | null>(null);
const [isLoading, setIsLoading] = useState(true);
const [error, setError] = useState<string | null>(null);
Expand Down Expand Up @@ -136,7 +138,15 @@ export function SettingsPanel() {
</div>
<input
type="number"
defaultValue={40}
value={weeklyHoursTarget}
min={1}
max={168}
onChange={(e) => {
const value = parseInt(e.target.value, 10);
if (!isNaN(value) && value >= 1 && value <= 168) {
updateSettings({ weeklyHoursTarget: value });
}
}}
Comment on lines 139 to +149
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The number input is missing a 'step' attribute. While the validation ensures only integers between 1 and 168 are accepted, without an explicit step attribute, some browsers may allow decimal inputs via the input spinner buttons. Consider adding step={1} to ensure the input only accepts whole numbers through all input methods.

Copilot uses AI. Check for mistakes.
className="w-20 rounded-lg border border-dark-600 bg-dark-800 px-3 py-2 text-dark-100 focus:outline-none focus:ring-2 focus:ring-thyme-500"
/>
Comment on lines 139 to 151
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The number input is missing an accessible label association. While there is descriptive text next to the input, screen readers cannot determine what the input controls. Add an aria-label or aria-labelledby attribute to the input element to associate it with the "Weekly Hours Target" label for better accessibility.

Copilot uses AI. Check for mistakes.
</div>
Expand All @@ -146,7 +156,12 @@ export function SettingsPanel() {
<p className="text-sm text-dark-400">Get reminded to fill in your timesheet</p>
</div>
<label className="relative inline-flex cursor-pointer items-center">
<input type="checkbox" defaultChecked className="peer sr-only" />
<input
type="checkbox"
checked={notificationsEnabled}
onChange={(e) => updateSettings({ notificationsEnabled: e.target.checked })}
className="peer sr-only"
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The custom checkbox toggle is missing an accessible label association. While there is descriptive text next to the toggle, screen readers cannot determine what the checkbox controls. Add an aria-label or aria-labelledby attribute to the checkbox input to associate it with the "Timesheet Reminders" label for better accessibility.

Suggested change
className="peer sr-only"
className="peer sr-only"
aria-label="Timesheet Reminders"

Copilot uses AI. Check for mistakes.
/>
<div className="peer h-6 w-11 rounded-full bg-dark-600 after:absolute after:left-[2px] after:top-[2px] after:h-5 after:w-5 after:rounded-full after:bg-white after:transition-all after:content-[''] peer-checked:bg-thyme-600 peer-checked:after:translate-x-full peer-focus:outline-none peer-focus:ring-2 peer-focus:ring-thyme-500"></div>
</label>
</div>
Expand Down