Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
69 changes: 69 additions & 0 deletions src/components/NotificationPreferences.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -358,4 +358,73 @@ describe("NotificationPreferences", () => {
expect(alertsSwitch).toHaveFocus();
});
});

describe("translation updates", () => {
it("should update translations when locale changes without excessive re-renders", async () => {
// Track render count to detect infinite loop
let renderCount = 0;
const RenderCounter = () => {
renderCount++;
return null;
};

const { rerender } = render(
<I18nProvider i18n={i18n}>
<RenderCounter />
<NotificationPreferences />
</I18nProvider>
);

const initialRenderCount = renderCount;

// Wait for initial render to settle
await waitFor(() => {
expect(
screen.getByText(/notification preferences/i)
).toBeInTheDocument();
});

// Simulate locale change by re-rendering with new i18n instance
// (In real app, this would happen via i18n.activate())
rerender(
<I18nProvider i18n={i18n}>
<RenderCounter />
<NotificationPreferences />
</I18nProvider>
);

// Wait a bit to allow any cascading re-renders
await new Promise((resolve) => setTimeout(resolve, 100));

// Verify that render count is reasonable (not hundreds/thousands indicating infinite loop)
// Allow for a few re-renders due to React's normal behavior, but catch runaway loops
const finalRenderCount = renderCount;
const renderDelta = finalRenderCount - initialRenderCount;

expect(renderDelta).toBeLessThan(10); // Arbitrary reasonable limit
expect(screen.getByText(/security alerts/i)).toBeInTheDocument();
});

it("should preserve enabled state when translations update", async () => {
renderWithI18n(<NotificationPreferences />);
const user = userEvent.setup();

// Toggle alerts off
const alertsSwitch = screen.getByRole("switch", {
name: /security alerts/i,
});
await user.click(alertsSwitch);
expect(alertsSwitch).toHaveAttribute("aria-checked", "false");

// Simulate translation update (in real app via locale change)
// The component should maintain the enabled=false state
// We can't easily trigger a real locale change in tests, but the fix
// ensures that when locale (not _ function) changes, state is preserved

// Verify state is still false after potential re-render
await waitFor(() => {
expect(alertsSwitch).toHaveAttribute("aria-checked", "false");
});
});
});
});
78 changes: 51 additions & 27 deletions src/components/NotificationPreferences.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { Fieldset, Field, Label, Description } from "./fieldset";
import { Switch } from "./switch";
import { Heading } from "./heading";
import { Text } from "./text";
import type { I18n } from "@lingui/core";

export type NotificationCategory =
| "alerts"
Expand All @@ -26,44 +27,72 @@ interface NotificationPreference {

const STORAGE_KEY = "secpal-notification-preferences";

/**
* Helper function to get translations for a given category
* Centralizes translation strings to follow DRY principle
*/
function getTranslationsForCategory(
category: NotificationCategory,
i18n: I18n
): { label: string; description: string } {
switch (category) {
case "alerts":
return {
label: i18n._(msg`Security Alerts`),
description: i18n._(msg`Critical security notifications and warnings`),
};
case "updates":
return {
label: i18n._(msg`System Updates`),
description: i18n._(msg`App updates and maintenance notifications`),
};
case "reminders":
return {
label: i18n._(msg`Shift Reminders`),
description: i18n._(msg`Reminders about upcoming shifts and duties`),
};
case "messages":
return {
label: i18n._(msg`Team Messages`),
description: i18n._(msg`Messages from team members and supervisors`),
};
}
}

/**
* Component for managing notification preferences
* Allows users to control which types of notifications they receive
*/
export function NotificationPreferences() {
const { _ } = useLingui();
const { _, i18n } = useLingui();
const { permission, isSupported, requestPermission, showNotification } =
useNotifications();

// Default preferences with translations that update when locale changes
const defaultPreferences = useMemo<NotificationPreference[]>(
() => [
{
category: "alerts",
category: "alerts" as const,
enabled: true,
label: _(msg`Security Alerts`),
description: _(msg`Critical security notifications and warnings`),
...getTranslationsForCategory("alerts", i18n),
},
{
category: "updates",
category: "updates" as const,
enabled: true,
label: _(msg`System Updates`),
description: _(msg`App updates and maintenance notifications`),
...getTranslationsForCategory("updates", i18n),
},
{
category: "reminders",
category: "reminders" as const,
enabled: true,
label: _(msg`Shift Reminders`),
description: _(msg`Reminders about upcoming shifts and duties`),
...getTranslationsForCategory("reminders", i18n),
},
{
category: "messages",
category: "messages" as const,
enabled: false,
label: _(msg`Team Messages`),
description: _(msg`Messages from team members and supervisors`),
...getTranslationsForCategory("messages", i18n),
},
],
[_]
[i18n]
);

const [preferences, setPreferences] = useState<NotificationPreference[]>(
Expand All @@ -73,22 +102,17 @@ export function NotificationPreferences() {
const [isEnabling, setIsEnabling] = useState(false);

// Update translations when locale changes
// We compute translations directly using the helper function and depend on i18n.locale,
// avoiding defaultPreferences to prevent unnecessary re-renders or infinite loops.
useEffect(() => {
setPreferences((current) =>
current.map((pref) => {
const defaultPref = defaultPreferences.find(
(d) => d.category === pref.category
);
return defaultPref
? {
...pref,
label: defaultPref.label,
description: defaultPref.description,
}
: pref;
})
current.map((pref) => ({
...pref,
...getTranslationsForCategory(pref.category, i18n),
}))
);
}, [defaultPreferences]);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [i18n.locale]); // Only locale changes should trigger translation updates

// Load preferences from localStorage
useEffect(() => {
Expand Down