fix(task-repeat-cfg): keep custom weekdays interactive (#8034)

This commit is contained in:
Johannes Millan 2026-06-05 20:12:34 +02:00 committed by GitHub
parent 7c03c84fad
commit 8e08b48a98
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 413 additions and 4 deletions

View file

@ -0,0 +1,192 @@
import { expect, test } from '../../fixtures/test.fixture';
import { type Page } from '@playwright/test';
import { openRecurDialog, saveRecurDialog } from '../../utils/recurring-task-helpers';
/**
* Bug: https://github.com/super-productivity/super-productivity/issues/8025
*
* In the CUSTOM recurring config, switching the "Recur cycle" away from Weekly
* (to Month/Year) and back to Weekly leaves the weekday checkboxes rendered but
* non-interactive: they "look enabled but are actually disabled". A click no
* longer updates the model, so the user's weekday selection is silently lost.
*
* Expected: after the Week -> Month -> Week round-trip the weekday checkboxes
* remain fully interactive: un-checking Monday persists, exactly as
* it would without the round-trip.
*
* This test reproduces the defect by toggling Monday off AFTER the round-trip
* and asserting the change survives a save (read back from the NgRx store). The
* weekday group is configured with `resetOnHide: false`, so preservation is the
* intended design; the failure is the lost interactivity, not a deliberate
* reset.
*/
const DIALOG = 'mat-dialog-container';
/** Switch the quick-setting select (first select in the dialog). */
const setQuickSetting = async (page: Page, optionLabel: RegExp): Promise<void> => {
const dialog = page.locator(DIALOG);
await dialog.locator('mat-select').first().click();
const option = page.locator('mat-option').filter({ hasText: optionLabel });
await expect(option).toBeVisible({ timeout: 5000 });
await option.click();
await expect(page.locator('mat-option')).toHaveCount(0, { timeout: 5000 });
};
/** Switch the CUSTOM "Recur cycle" select (Day / Week / Month / Year). */
const setRepeatCycle = async (page: Page, optionLabel: RegExp): Promise<void> => {
const dialog = page.locator(DIALOG);
const cycleSelect = dialog.locator('.repeat-cycle mat-select').first();
await expect(cycleSelect).toBeVisible({ timeout: 5000 });
await cycleSelect.click();
const option = page.locator('mat-option').filter({ hasText: optionLabel });
await expect(option).toBeVisible({ timeout: 5000 });
await option.click();
await expect(page.locator('mat-option')).toHaveCount(0, { timeout: 5000 });
};
type WeekdayKey =
| 'monday'
| 'tuesday'
| 'wednesday'
| 'thursday'
| 'friday'
| 'saturday'
| 'sunday';
type PersistedRepeatCfgSnapshot = {
quickSetting: string | null;
repeatCycle: string | null;
weekday: boolean | null;
};
/** Read a recurrence snapshot for the repeat config created for the given title. */
const getPersistedRepeatCfgSnapshot = async (
page: Page,
title: string,
weekday: WeekdayKey,
): Promise<PersistedRepeatCfgSnapshot> =>
page.evaluate(
({ taskTitle, weekdayKey }: { taskTitle: string; weekdayKey: WeekdayKey }) => {
type RepeatCfgLike = {
quickSetting?: string | null;
repeatCycle?: string | null;
title?: string | null;
} & Partial<Record<WeekdayKey, boolean>>;
type StoreState = {
taskRepeatCfg?: { entities?: Record<string, RepeatCfgLike | undefined> };
};
type StoreLike = {
subscribe: (next: (s: StoreState) => void) => { unsubscribe: () => void };
};
const store = (window as unknown as { __e2eTestHelpers?: { store?: StoreLike } })
.__e2eTestHelpers?.store;
if (!store) {
throw new Error('__e2eTestHelpers.store missing');
}
let latest: StoreState | undefined;
store
.subscribe((s) => {
latest = s;
})
.unsubscribe();
const cfg = Object.values(latest?.taskRepeatCfg?.entities ?? {}).find((c) =>
c?.title?.includes(taskTitle),
);
return {
quickSetting: cfg?.quickSetting ?? null,
repeatCycle: cfg?.repeatCycle ?? null,
weekday: cfg ? (cfg[weekdayKey] ?? null) : null,
};
},
{ taskTitle: title, weekdayKey: weekday },
);
test.describe('Recurring Task - Custom weekday checkboxes frozen after cycle round-trip (#8025)', () => {
test('should keep weekday checkboxes interactive after switching cycle to Month and back to Week', async ({
page,
workViewPage,
taskPage,
testPrefix,
}) => {
const taskTitle = `${testPrefix}-WeekdayFrozen8025`;
await workViewPage.waitForTaskList();
await workViewPage.addTask(taskTitle);
const task = taskPage.getTaskByText(taskTitle).first();
await expect(task).toBeVisible({ timeout: 10000 });
await taskPage.openTaskDetail(task);
const dialog = await openRecurDialog(page);
// Enter CUSTOM mode; repeatCycle defaults to WEEKLY with Mon-Fri checked.
await setQuickSetting(page, /Custom recurring config/i);
const mondayCheckbox = dialog
.locator('.weekdays mat-checkbox')
.filter({ hasText: /Monday/i });
const mondayInput = mondayCheckbox.locator('input[type="checkbox"]');
await expect(mondayInput).toBeChecked(); // sanity: default Weekly has Monday on
// The round-trip that triggers the bug: Week -> Month -> Week.
await setRepeatCycle(page, /^Month$/);
await setRepeatCycle(page, /^Week$/);
// After the round-trip the checkbox is re-rendered. Try to un-check Monday.
await expect(mondayCheckbox).toBeVisible({ timeout: 5000 });
await mondayCheckbox.click();
// Save and read the persisted config back from the store. With the bug the
// click never reached the model, so `monday` is still `true`.
await saveRecurDialog(page);
await expect
.poll(() => getPersistedRepeatCfgSnapshot(page, taskTitle, 'monday'), {
timeout: 10000,
})
.toEqual({
quickSetting: 'CUSTOM',
repeatCycle: 'WEEKLY',
weekday: false,
});
});
test('should keep weekday checkboxes interactive after switching away from CUSTOM and back', async ({
page,
workViewPage,
taskPage,
testPrefix,
}) => {
const taskTitle = `${testPrefix}-CustomRoundTrip8025`;
await workViewPage.waitForTaskList();
await workViewPage.addTask(taskTitle);
const task = taskPage.getTaskByText(taskTitle).first();
await expect(task).toBeVisible({ timeout: 10000 });
await taskPage.openTaskDetail(task);
const dialog = await openRecurDialog(page);
await setQuickSetting(page, /Custom recurring config/i);
await setQuickSetting(page, /Every day/i);
await setQuickSetting(page, /Custom recurring config/i);
await setRepeatCycle(page, /^Week$/);
const tuesdayCheckbox = dialog
.locator('.weekdays mat-checkbox')
.filter({ hasText: /Tuesday/i });
const tuesdayInput = tuesdayCheckbox.locator('input[type="checkbox"]');
await expect(tuesdayInput).toBeChecked();
await tuesdayCheckbox.click();
await saveRecurDialog(page);
await expect
.poll(() => getPersistedRepeatCfgSnapshot(page, taskTitle, 'tuesday'), {
timeout: 10000,
})
.toEqual({
quickSetting: 'CUSTOM',
repeatCycle: 'WEEKLY',
weekday: false,
});
});
});

View file

@ -32,6 +32,16 @@
[model]="repeatCfg()"
></formly-form>
@if (isWeekdaySelectionInvalid()) {
<div
id="weekday-error"
class="weekday-error"
role="alert"
>
{{ T.F.TASK_REPEAT.F.WEEKDAY_REQUIRED | translate }}
</div>
}
<collapsible [title]="T.F.TASK_REPEAT.D_EDIT.ADVANCED_CFG | translate">
<chip-list-input
(addItem)="addTag($event)"
@ -93,8 +103,14 @@
}
<button
[disabled]="!formGroup1().valid || !formGroup2().valid || isLoading()"
[disabled]="
!formGroup1().valid ||
!formGroup2().valid ||
isLoading() ||
isWeekdaySelectionInvalid()
"
[attr.aria-label]="T.G.SAVE | translate"
[attr.aria-describedby]="isWeekdaySelectionInvalid() ? 'weekday-error' : null"
class="icon-on-small"
color="primary"
mat-raised-button

View file

@ -70,6 +70,21 @@ mat-dialog-actions {
min-width: 0;
}
}
// Visibility toggle for the weekday group, which must stay mounted (#8025):
// hiding via CSS keeps formly from destroying/recreating the child views, which
// would otherwise break the checkbox/FormControl wiring after a cycle round-trip.
:host::ng-deep .repeat-cfg-hidden {
display: none;
}
// Inline error shown when a CUSTOM weekly config has no weekday selected (#8025).
.weekday-error {
color: var(--c-warn);
font-size: var(--font-size-sm);
margin: 0 0 var(--s2);
margin-inline-start: var(--s2);
}
:host::ng-deep .repeat-cycle {
display: flex;

View file

@ -535,4 +535,87 @@ describe('DialogEditTaskRepeatCfgComponent', () => {
// which only happens after repeatCfgInitial is set
}));
});
describe('isWeekdaySelectionInvalid (issue #8025)', () => {
const baseCfg = {
...DEFAULT_TASK_REPEAT_CFG,
monday: false,
tuesday: false,
wednesday: false,
thursday: false,
friday: false,
saturday: false,
sunday: false,
};
it('should be true for a CUSTOM weekly config with no weekday selected', async () => {
const fixture = await setupTestBed({ task: mockTask });
const component = fixture.componentInstance;
component.repeatCfg.set({
...baseCfg,
quickSetting: 'CUSTOM',
repeatCycle: 'WEEKLY',
});
expect(component.isWeekdaySelectionInvalid()).toBe(true);
});
it('should be false once at least one weekday is selected', async () => {
const fixture = await setupTestBed({ task: mockTask });
const component = fixture.componentInstance;
component.repeatCfg.set({
...baseCfg,
quickSetting: 'CUSTOM',
repeatCycle: 'WEEKLY',
wednesday: true,
});
expect(component.isWeekdaySelectionInvalid()).toBe(false);
});
it('should be false for non-weekly cycles even with no weekday selected', async () => {
const fixture = await setupTestBed({ task: mockTask });
const component = fixture.componentInstance;
component.repeatCfg.set({
...baseCfg,
quickSetting: 'CUSTOM',
repeatCycle: 'MONTHLY',
});
expect(component.isWeekdaySelectionInvalid()).toBe(false);
});
it('should be false for non-CUSTOM quick settings (e.g. DAILY)', async () => {
const fixture = await setupTestBed({ task: mockTask });
const component = fixture.componentInstance;
component.repeatCfg.set({
...baseCfg,
quickSetting: 'DAILY',
repeatCycle: 'WEEKLY',
});
expect(component.isWeekdaySelectionInvalid()).toBe(false);
});
it('should block direct save when a CUSTOM weekly config has no weekday selected', async () => {
const fixture = await setupTestBed({ task: mockTask });
const component = fixture.componentInstance;
component.repeatCfg.set({
...baseCfg,
quickSetting: 'CUSTOM',
repeatCycle: 'WEEKLY',
});
component.save();
expect(mockTaskRepeatCfgService.addTaskRepeatCfgToTask).not.toHaveBeenCalled();
expect(mockTaskRepeatCfgService.updateTaskRepeatCfg).not.toHaveBeenCalled();
expect(mockDialogRef.close).not.toHaveBeenCalled();
});
});
});

View file

@ -64,6 +64,18 @@ const RELEVANT_KEYS_FOR_UPDATE_ALL_TASKS: (keyof TaskRepeatCfgCopy)[] = [
'tagIds',
];
// A CUSTOM weekly recurrence with no weekday checked never produces an
// occurrence, so it must be blocked at save time (#8025).
const WEEKDAY_KEYS: (keyof TaskRepeatCfgCopy)[] = [
'monday',
'tuesday',
'wednesday',
'thursday',
'friday',
'saturday',
'sunday',
];
// TASK_REPEAT_CFG_FORM_CFG
@Component({
selector: 'dialog-edit-task-repeat-cfg',
@ -113,6 +125,17 @@ export class DialogEditTaskRepeatCfgComponent {
return false;
});
// A CUSTOM weekly config with zero weekdays selected would never recur;
// surface it as a blocking validation error (#8025). Derived from the
// `repeatCfg` signal so it re-evaluates on every checkbox toggle.
isWeekdaySelectionInvalid = computed(() => {
const cfg = this.repeatCfg();
if (cfg.quickSetting !== 'CUSTOM' || cfg.repeatCycle !== 'WEEKLY') {
return false;
}
return !WEEKDAY_KEYS.some((day) => cfg[day]);
});
repeatCfgId = computed(() => {
const cfg = this.repeatCfg();
if ('id' in cfg && cfg.id) {
@ -292,6 +315,11 @@ export class DialogEditTaskRepeatCfgComponent {
return;
}
// Enter-key submit bypasses the disabled Save button, so re-check here (#8025).
if (this.isWeekdaySelectionInvalid()) {
return;
}
const currentRepeatCfg = this.repeatCfg();
// workaround for formly not always updating hidden fields correctly (in time??)

View file

@ -73,4 +73,59 @@ describe('TaskRepeatCfgFormConfig', () => {
expect(remindAtField?.templateOptions?.required).toBe(true);
});
});
describe('weekdays group visibility (issue #8025)', () => {
const repeatContainer = TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG.find(
(field) => field.fieldGroupClassName === 'repeat-config-container',
);
const weekdaysGroup = repeatContainer?.fieldGroup?.find(
(field) => field.fieldGroupClassName === 'weekdays',
);
const WEEKDAY_KEYS = [
'monday',
'tuesday',
'wednesday',
'thursday',
'friday',
'saturday',
'sunday',
];
it('should locate the weekdays group', () => {
expect(weekdaysGroup).toBeDefined();
expect(weekdaysGroup?.fieldGroup?.length).toBe(7);
});
// Regression guard for #8025: a `hideExpression` makes formly destroy and
// recreate the checkbox views on every cycle switch, which breaks their
// wiring to the FormControls (clicks stop updating the model after a
// Week -> Month -> Week round-trip). Visibility must be driven by CSS instead.
it('should NOT use hideExpression (it would re-freeze the checkboxes)', () => {
expect(weekdaysGroup?.hideExpression).toBeUndefined();
});
it('should toggle visibility via a dynamic className bound to repeatCycle', () => {
const className = weekdaysGroup?.expressionProperties?.['className'] as (model: {
repeatCycle: string;
}) => string;
expect(className).toEqual(jasmine.any(Function));
expect(className({ repeatCycle: 'WEEKLY' })).toBe('');
expect(className({ repeatCycle: 'MONTHLY' })).toBe('repeat-cfg-hidden');
expect(className({ repeatCycle: 'YEARLY' })).toBe('repeat-cfg-hidden');
expect(className({ repeatCycle: 'DAILY' })).toBe('repeat-cfg-hidden');
});
// The group stays mounted, but the CUSTOM container above it still hides via
// hideExpression. Each checkbox needs resetOnHide:false so the selection
// survives a quickSetting != CUSTOM round-trip instead of being wiped.
it('should keep every weekday checkbox value across hide (resetOnHide:false)', () => {
WEEKDAY_KEYS.forEach((key) => {
const field = weekdaysGroup?.fieldGroup?.find((f) => f.key === key);
expect(field).withContext(`weekday field "${key}" exists`).toBeDefined();
expect(field?.resetOnHide)
.withContext(`weekday field "${key}" resetOnHide`)
.toBe(false);
});
});
});
});

View file

@ -143,13 +143,25 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [
],
},
{
// Hide via a dynamic CSS class instead of `hideExpression`. With formly's
// default `lazyRender`, hiding a field group destroys its child views and
// recreates them on re-show, and the recreated mat-checkboxes lose their
// wiring to the (re-registered) FormControls. After a cycle round-trip
// (Week -> Month -> Week) the checkboxes then look enabled but are inert:
// clicks no longer update the model (#8025). Keeping the group mounted and
// toggling only its visibility preserves the control/view binding.
// `resetOnHide: false` on each checkbox keeps the selection when the
// CUSTOM container itself is hidden (quickSetting != CUSTOM).
fieldGroupClassName: 'weekdays',
resetOnHide: false,
hideExpression: (model: any) => model.repeatCycle !== 'WEEKLY',
expressionProperties: {
className: (model: TaskRepeatCfg) =>
model.repeatCycle === 'WEEKLY' ? '' : 'repeat-cfg-hidden',
},
fieldGroup: [
{
key: 'monday',
type: 'checkbox',
resetOnHide: false,
templateOptions: {
label: T.F.TASK_REPEAT.F.MONDAY,
},
@ -157,6 +169,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [
{
key: 'tuesday',
type: 'checkbox',
resetOnHide: false,
templateOptions: {
label: T.F.TASK_REPEAT.F.TUESDAY,
},
@ -164,6 +177,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [
{
key: 'wednesday',
type: 'checkbox',
resetOnHide: false,
templateOptions: {
label: T.F.TASK_REPEAT.F.WEDNESDAY,
},
@ -171,6 +185,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [
{
key: 'thursday',
type: 'checkbox',
resetOnHide: false,
templateOptions: {
label: T.F.TASK_REPEAT.F.THURSDAY,
},
@ -178,6 +193,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [
{
key: 'friday',
type: 'checkbox',
resetOnHide: false,
templateOptions: {
label: T.F.TASK_REPEAT.F.FRIDAY,
},
@ -185,6 +201,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [
{
key: 'saturday',
type: 'checkbox',
resetOnHide: false,
templateOptions: {
label: T.F.TASK_REPEAT.F.SATURDAY,
},
@ -192,6 +209,7 @@ export const TASK_REPEAT_CFG_ESSENTIAL_FORM_CFG: FormlyFieldConfig[] = [
{
key: 'sunday',
type: 'checkbox',
resetOnHide: false,
templateOptions: {
label: T.F.TASK_REPEAT.F.SUNDAY,
},

View file

@ -1977,6 +1977,7 @@ const T = {
WEDNESDAY: 'F.TASK_REPEAT.F.WEDNESDAY',
WEEK_OF_MONTH: 'F.TASK_REPEAT.F.WEEK_OF_MONTH',
WEEKDAY: 'F.TASK_REPEAT.F.WEEKDAY',
WEEKDAY_REQUIRED: 'F.TASK_REPEAT.F.WEEKDAY_REQUIRED',
},
SNACK_REPEAT_DIALOG_FAIL: 'F.TASK_REPEAT.SNACK_REPEAT_DIALOG_FAIL',
},

View file

@ -1923,7 +1923,8 @@
"WAIT_FOR_COMPLETION_DESCRIPTION": "Prevents multiple pending recurring tasks from piling up. The next instance will only be created once the current task is completed",
"WEDNESDAY": "Wednesday",
"WEEK_OF_MONTH": "Week of month",
"WEEKDAY": "Weekday"
"WEEKDAY": "Weekday",
"WEEKDAY_REQUIRED": "Select at least one weekday"
},
"SNACK_REPEAT_DIALOG_FAIL": "Could not open repeat config dialog. You can configure it from the task menu."
},