Skip to content

Commit 6abb4ae

Browse files
akolsonclaude
andcommitted
Fix mark-all-present switch requiring two clicks after modal cancel
KSwitch optimistically toggles its internal state on click. When the "mark all present" modal was cancelled, allPresent stayed false (unchanged), so Vue never re-rendered the switch — leaving it visually on. The next click then fired @change(false) (a no-op) instead of reopening the modal. Introduce pendingMarkAll ref set true when the modal opens and false when cancelled or confirmed. The switch binds to markAllSwitchValue (allPresent || pendingMarkAll), so cancelling creates a real true→false prop change that forces the KSwitch to re-render back to unchecked. Adds a regression test reproducing the exact cancel→single-click sequence. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b4a0b21 commit 6abb4ae

3 files changed

Lines changed: 45 additions & 3 deletions

File tree

kolibri/plugins/coach/frontend/composables/useAttendanceForm.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export default function useAttendanceForm({ hasChanges, markClean, submitting, o
2626
const previouslyEnrolledMap = ref({});
2727
const enrolledLearnerIds = ref(null);
2828
const showMarkAllModal = ref(false);
29+
const pendingMarkAll = ref(false);
2930
const pendingRoute = ref(null);
3031

3132
const backRoute = computed(() => ({
@@ -95,6 +96,12 @@ export default function useAttendanceForm({ hasChanges, markClean, submitting, o
9596
sortedLearners.value.length > 0 && currentPresentCount.value === sortedLearners.value.length,
9697
);
9798

99+
// The switch should appear checked if all learners are genuinely present, OR if the
100+
// user has clicked "mark all present" and is seeing the confirmation modal (pendingMarkAll).
101+
// When the modal is canceled, pendingMarkAll resets to false, which changes this computed
102+
// from true → false and gives Vue a real prop change to re-render the KSwitch correctly.
103+
const markAllPresent = computed(() => allPresent.value || pendingMarkAll.value);
104+
98105
function setAllLearners(value) {
99106
const newMap = {};
100107
sortedLearners.value.forEach(l => {
@@ -106,6 +113,7 @@ export default function useAttendanceForm({ hasChanges, markClean, submitting, o
106113

107114
function handleMarkAllChange(checked) {
108115
if (checked) {
116+
pendingMarkAll.value = true;
109117
showMarkAllModal.value = true;
110118
} else {
111119
setAllLearners(false);
@@ -114,10 +122,12 @@ export default function useAttendanceForm({ hasChanges, markClean, submitting, o
114122

115123
function confirmMarkAll() {
116124
setAllLearners(true);
125+
pendingMarkAll.value = false;
117126
showMarkAllModal.value = false;
118127
}
119128

120129
function cancelMarkAll() {
130+
pendingMarkAll.value = false;
121131
showMarkAllModal.value = false;
122132
}
123133

@@ -167,6 +177,7 @@ export default function useAttendanceForm({ hasChanges, markClean, submitting, o
167177
absentCount,
168178
currentAbsentCount,
169179
allPresent,
180+
markAllPresent,
170181
showMarkAllModal,
171182
pendingRoute,
172183
isPresent,

kolibri/plugins/coach/frontend/views/attendance/AttendanceFormTable.vue

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
<KSwitch
3232
name="mark-all-present"
3333
:ariaLabelledBy="'mark-all-present-label'"
34-
:value="allPresent"
34+
:value="markAllSwitchValue"
3535
@change="handleMarkAllChange"
3636
/>
3737
</div>
@@ -200,7 +200,7 @@
200200
const {
201201
sortedLearners,
202202
sortedPreviouslyEnrolled,
203-
allPresent,
203+
markAllPresent: markAllSwitchValue,
204204
presentCount: presentCountValue,
205205
absentCount: absentCountValue,
206206
currentAbsentCount: currentAbsentCountValue,
@@ -250,7 +250,7 @@
250250
confirmButtonStyles,
251251
allItems,
252252
sortedLearners,
253-
allPresent,
253+
markAllSwitchValue,
254254
presentCount: presentCountValue,
255255
absentCount: absentCountValue,
256256
currentAbsentCount: currentAbsentCountValue,

kolibri/plugins/coach/frontend/views/attendance/__tests__/AttendancePages.spec.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,37 @@ describe('AttendanceNewPage', () => {
300300
expect(buttons.length).toBe(2);
301301
});
302302

303+
it('re-opens modal on a single click after "Go back" was clicked in the mark-all modal', async () => {
304+
// Regression: after cancelling the modal, the KSwitch stayed visually "on" even though
305+
// allPresent was false. The next click fired @change(false) instead of @change(true),
306+
// so the modal never re-opened — requiring a second click to trigger it again.
307+
renderNewPage();
308+
await waitFor(() => {
309+
expect(screen.getByText('Alice')).toBeInTheDocument();
310+
});
311+
312+
// First click — opens modal
313+
await fireEvent.click(getMarkAllSwitch());
314+
await waitFor(() => {
315+
expect(screen.getByRole('dialog')).toBeInTheDocument();
316+
});
317+
318+
// Click "Go back" inside the modal
319+
await fireEvent.click(screen.getByRole('button', { name: 'Go back' }));
320+
await waitFor(() => {
321+
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
322+
});
323+
324+
// Switch should be visually unchecked after cancellation
325+
expect(getMarkAllSwitch().checked).toBe(false);
326+
327+
// Second click — should open the modal again in a single click
328+
await fireEvent.click(getMarkAllSwitch());
329+
await waitFor(() => {
330+
expect(screen.getByRole('dialog')).toBeInTheDocument();
331+
});
332+
});
333+
303334
it('marks all learners present after confirming modal', async () => {
304335
renderNewPage();
305336
await waitFor(() => {

0 commit comments

Comments
 (0)