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
19 changes: 19 additions & 0 deletions frontend/src/__tests__/settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1584,6 +1584,25 @@ describe('Settings Module', () => {
expect(errEl!.textContent).toBe(msg);
});

it('clearing the default coverage field shows the inline error instead of passing as 0 (#522)', () => {
const input = document.getElementById('setting-default-coverage') as HTMLInputElement;
input.value = '';
fire(input, 'input');

expect(input.getAttribute('aria-invalid')).toBe('true');
const errEl = document.getElementById('setting-default-coverage-range-error');
expect(errEl!.classList.contains('hidden')).toBe(false);
expect(errEl!.textContent).toBe('Must be a whole number between 0 and 100');
});

it('clearing an optional numeric field stays without an inline error', () => {
const input = document.getElementById('setting-notification-days') as HTMLInputElement;
input.value = '';
fire(input, 'input');

expect(input.getAttribute('aria-invalid')).toBeNull();
});

it('typing a fractional value into notification-days is rejected inline', () => {
const input = document.getElementById('setting-notification-days') as HTMLInputElement;
input.value = '1.5';
Expand Down
11 changes: 7 additions & 4 deletions frontend/src/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,11 +261,12 @@ function byId<T extends HTMLElement>(id: string): T | null {
* The element's min/max attributes are the source of truth so the
* range stays in sync with the HTML. An empty value clears the error
* (the input may have its own "required" enforcement; we don't fight
* that here).
* that here). Pass `required` for a field whose empty value would
* save as 0 (the save path rejects it too), so the error shows inline.
*/
function wireInlineRangeValidation(
inputId: string,
{ signal, requireInteger = false }: { signal?: AbortSignal; requireInteger?: boolean } = {},
{ signal, requireInteger = false, required = false }: { signal?: AbortSignal; requireInteger?: boolean; required?: boolean } = {},
): void {
const input = document.getElementById(inputId) as HTMLInputElement | null;
if (!input) return;
Expand Down Expand Up @@ -312,13 +313,15 @@ function wireInlineRangeValidation(

const check = (): void => {
const raw = input.value.trim();
if (raw === '') {
if (raw === '' && !required) {
input.removeAttribute('aria-invalid');
error.classList.add('hidden');
return;
}
// Number('') is 0, so a required empty field is invalid explicitly.
const n = Number(raw);
const invalid =
raw === '' ||
!Number.isFinite(n) ||
(requireInteger && !Number.isInteger(n)) ||
n < min ||
Expand Down Expand Up @@ -2703,7 +2706,7 @@ export function setupSettingsHandlers(signal?: AbortSignal): void {
// across Settings.
wireInlineRangeValidation('setting-notification-days', { signal, requireInteger: true });
wireInlineRangeValidation('setting-recs-stale-hours', { signal, requireInteger: true });
wireInlineRangeValidation('setting-default-coverage', { signal, requireInteger: true });
wireInlineRangeValidation('setting-default-coverage', { signal, requireInteger: true, required: true });
wireInlineRangeValidation('setting-grace-aws', { signal, requireInteger: true });
wireInlineRangeValidation('setting-grace-azure', { signal, requireInteger: true });
wireInlineRangeValidation('setting-grace-gcp', { signal, requireInteger: true });
Expand Down
Loading