From 6f06e8924977265f16543292e28ee8d564c0a67a Mon Sep 17 00:00:00 2001 From: Gabriel Morador Date: Mon, 10 Aug 2026 01:02:59 -0300 Subject: [PATCH 1/3] Update the Checkpoint step copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match the wording design asked for: - "Manual" becomes "Manual reveal", with a subtitle explaining that the instructor clicks a button to reveal student annotations. The subtitle is rendered by `RadioGroup.Radio`, wrapped in `small` like the other radio subtitles in the file picker (see `AutoGradingConfigurator`). - "More coming soon" becomes "More options coming". - The note below the radios now defines what a Checkpoint is instead of describing the manual reveal. The new note holds for any checkpoint type, so it no longer hangs off `showManualNote`. That condition was already dead — `CheckpointType` has a single member, so it never evaluated false, which is why it needed an eslint-disable for `no-unnecessary-condition`. No style changes: the copy is the only thing that moves. --- .../components/CheckpointSelector.tsx | 34 +++++++++++-------- .../test/CheckpointSelector-test.js | 17 ++-------- 2 files changed, 22 insertions(+), 29 deletions(-) diff --git a/lms/static/scripts/frontend_apps/components/CheckpointSelector.tsx b/lms/static/scripts/frontend_apps/components/CheckpointSelector.tsx index 7a6f70c47e..4a8b75d06d 100644 --- a/lms/static/scripts/frontend_apps/components/CheckpointSelector.tsx +++ b/lms/static/scripts/frontend_apps/components/CheckpointSelector.tsx @@ -28,12 +28,6 @@ export default function CheckpointSelector({ }: CheckpointSelectorProps) { const headingId = useId(); - // The note below is specific to the "manual" reveal, so it only shows for that - // option. `selected` is currently always 'manual' (the only enabled option), - // but this keeps the association explicit for when more types are added. - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - const showManualNote = selected === 'manual'; - return (

@@ -52,18 +46,28 @@ export default function CheckpointSelector({ } }} > - Manual + + Instructor clicks a button to reveal student annotations + + } + > + Manual reveal + - More coming soon + More options coming - {showManualNote && ( - // No color class: inherits the base text color (black) per design. -

- Students will see when the settings have changed from - “Hide” to “Reveal” in their notifications. -

- )} + {/* No color class: inherits the base text color (black) per design. */} +

+ A Checkpoint is the moment when student annotations switch from hidden + to visible. Before the Checkpoint, students write without seeing each + other’s work. +

); } diff --git a/lms/static/scripts/frontend_apps/components/test/CheckpointSelector-test.js b/lms/static/scripts/frontend_apps/components/test/CheckpointSelector-test.js index 51a12eb35c..ada1d56b8a 100644 --- a/lms/static/scripts/frontend_apps/components/test/CheckpointSelector-test.js +++ b/lms/static/scripts/frontend_apps/components/test/CheckpointSelector-test.js @@ -47,23 +47,12 @@ describe('CheckpointSelector', () => { assert.calledWith(fakeOnChange, 'automatic'); }); - it('shows the reveal note when "manual" is selected', () => { - const wrapper = createComponent('manual'); + it('explains what a checkpoint is', () => { + const wrapper = createComponent(); assert.include( wrapper.text(), - 'Students will see when the settings have changed', - ); - }); - - it('hides the reveal note when a non-manual option is selected', () => { - // `selected` is typed `'manual'` today (the only option), so this exercises - // the conditional that will matter once more checkpoint types exist. - const wrapper = createComponent('more'); - - assert.notInclude( - wrapper.text(), - 'Students will see when the settings have changed', + 'A Checkpoint is the moment when student annotations switch from hidden to visible', ); }); From 7c2bfa2f82ec877931cd692ecb786d7c2d964e89 Mon Sep 17 00:00:00 2001 From: Gabriel Morador Date: Mon, 10 Aug 2026 01:03:09 -0300 Subject: [PATCH 2/3] Skip the due-date step for now The date the step collects has no effect yet, so offering it suggests the assignment does something it doesn't. Michael asked for it to be hidden until due dates are wired up. "Next" on the checkpoint step now completes the workflow instead of advancing to the due date, which leaves the step unreachable. Nothing else was removed: the state, the ref, the selector, its title and the `'due-date'` member of the step union all stay, and the original transition sits commented out right above the temporary one. Restoring it, and the blocks commented out in the test, brings the step back. The now-unreachable validation guard gets an `istanbul ignore` so its `return` doesn't break the 100% statement threshold. `DueDateSelector` and its tests are untouched. --- .../components/FilePickerApp.tsx | 11 +- .../components/test/FilePickerApp-test.js | 293 +++++++++--------- 2 files changed, 157 insertions(+), 147 deletions(-) diff --git a/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx b/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx index 6785f5f9b3..906294f1df 100644 --- a/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx +++ b/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx @@ -320,6 +320,7 @@ export default function FilePickerApp({ onSubmit }: FilePickerAppProps) { // from the legal "left blank", and a complete one can have fallen into // the past. Only the selector can tell; it shows the reason for any // rejection itself. + /* istanbul ignore next: unreachable while the due-date step is skipped */ if ( workflowStep === 'due-date' && dueDateSelectorRef.current && @@ -327,10 +328,18 @@ export default function FilePickerApp({ onSubmit }: FilePickerAppProps) { ) { return; } + // TEMPORARY, at Michael's request: the due-date step is skipped because the + // date it collects has no effect yet, and offering it suggests the + // assignment does something it doesn't. Nothing else about the step was + // removed — to turn it back on, restore the commented-out line below in + // place of the one under it, and the blocks commented out in + // `FilePickerApp-test`. + // // From 'checkpoint' the next step is 'due-date'; from 'due-date' (the last // step) the workflow is done. The 'assignment-type' step has no "Next" — it // advances directly on selection (see `selectAssignmentType`). - setWorkflowStep(step => (step === 'checkpoint' ? 'due-date' : 'done')); + // setWorkflowStep(step => (step === 'checkpoint' ? 'due-date' : 'done')); + setWorkflowStep('done'); }; // Pick an assignment type in the first workflow step. Unlike the later steps, diff --git a/lms/static/scripts/frontend_apps/components/test/FilePickerApp-test.js b/lms/static/scripts/frontend_apps/components/test/FilePickerApp-test.js index 0bbebaa528..b955b830e8 100644 --- a/lms/static/scripts/frontend_apps/components/test/FilePickerApp-test.js +++ b/lms/static/scripts/frontend_apps/components/test/FilePickerApp-test.js @@ -140,39 +140,42 @@ describe('FilePickerApp', () => { }); } - function setDueDate(wrapper, date) { - interact(wrapper, () => { - wrapper.find('DueDateSelector').first().props().onChange(date); - }); - } - - /** - * Stand in for `DueDateSelector`'s `validate`. - * - * `DueDateSelector` is mocked here, so the real fields — and the checks - * the real component runs over them — never exist. The mock still - * receives `selectorRef`, so populating the handle decides what the - * parent hears when it validates the step. - */ - function setDueDateValidity(wrapper, valid) { - const { selectorRef } = wrapper.find('DueDateSelector').first().props(); - selectorRef.current = { validate: sinon.stub().returns(valid) }; - return selectorRef.current; - } - - /** - * Local `datetime-local` string (`YYYY-MM-DDTHH:MM`) `days` from now - * (negative for the past). - */ - function dueDateFromNow(days) { - const date = new Date(); - date.setDate(date.getDate() + days); - const pad = n => String(n).padStart(2, '0'); - return ( - `${date.getFullYear()}-${pad(date.getMonth() + 1)}-${pad(date.getDate())}` + - `T${pad(date.getHours())}:${pad(date.getMinutes())}` - ); - } + // Commented out while the due-date step is skipped (see + // `goToNextWorkflowStep`). Restore alongside the step. + // + // function setDueDate(wrapper, date) { + // interact(wrapper, () => { + // wrapper.find('DueDateSelector').first().props().onChange(date); + // }); + // } + // + // /** + // * Stand in for `DueDateSelector`'s `validate`. + // * + // * `DueDateSelector` is mocked here, so the real fields — and the checks + // * the real component runs over them — never exist. The mock still + // * receives `selectorRef`, so populating the handle decides what the + // * parent hears when it validates the step. + // */ + // function setDueDateValidity(wrapper, valid) { + // const { selectorRef } = wrapper.find('DueDateSelector').first().props(); + // selectorRef.current = { validate: sinon.stub().returns(valid) }; + // return selectorRef.current; + // } + // + // /** + // * Local `datetime-local` string (`YYYY-MM-DDTHH:MM`) `days` from now + // * (negative for the past). + // */ + // function dueDateFromNow(days) { + // const date = new Date(); + // date.setDate(date.getDate() + days); + // const pad = n => String(n).padStart(2, '0'); + // return ( + // `${date.getFullYear()}-${pad(date.getMonth() + 1)}-${pad(date.getDate())}` + + // `T${pad(date.getHours())}:${pad(date.getMinutes())}` + // ); + // } it('does not show the workflow when only one type is available', () => { fakeConfig.filePicker.assignmentTypes = ['reading']; @@ -219,7 +222,7 @@ describe('FilePickerApp', () => { assert.isTrue(wrapper.exists('ContentSelector')); }); - it('walks through checkpoint and due-date steps for "Hide & Reveal"', () => { + it('walks through the checkpoint step for "Hide & Reveal"', () => { fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; const wrapper = renderFilePicker(); @@ -231,64 +234,64 @@ describe('FilePickerApp', () => { assert.isFalse(wrapper.exists('ContentSelector')); clickNext(wrapper); - // Due-date step. - assert.isTrue(wrapper.exists('DueDateSelector')); - assert.isFalse(wrapper.exists('ContentSelector')); - clickNext(wrapper); - - // Regular flow takes over. + // Regular flow takes over: the due-date step that used to sit here is + // skipped for now (see `goToNextWorkflowStep`). assert.isFalse(wrapper.exists('DueDateSelector')); assert.isTrue(wrapper.exists('ContentSelector')); }); - it('blocks the due-date step while the selector reports an invalid value', () => { - fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; - const wrapper = renderFilePicker(); - - selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint - clickNext(wrapper); // -> due-date - assert.isTrue(wrapper.exists('DueDateSelector')); - - // Whatever the reason — a half-entered value, or a complete one that is - // no longer in the future — the selector shows it inline; the parent - // only acts on the verdict. - const handle = setDueDateValidity(wrapper, false); - - clickNext(wrapper); - assert.isTrue(wrapper.exists('DueDateSelector')); - assert.isFalse(wrapper.exists('ContentSelector')); - assert.called(handle.validate); - }); - - it('anchors the earliest selectable due date to the present', () => { - fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; - const wrapper = renderFilePicker(); - - selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint - clickNext(wrapper); // -> due-date - - // The selector rejects anything below `min` (past dates, past times on - // the current day); pinning `min` to "now" is the parent's half of the - // "no due dates in the past" rule. - const { min } = wrapper.find('DueDateSelector').first().props(); - assert.isTrue(min <= dueDateFromNow(0)); - assert.isTrue(min > dueDateFromNow(-1)); - }); - - it('leaves the due-date step when the selector reports a valid value', () => { - fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; - const wrapper = renderFilePicker(); - - selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint - clickNext(wrapper); // -> due-date - - setDueDateValidity(wrapper, true); - setDueDate(wrapper, dueDateFromNow(7)); - - clickNext(wrapper); - assert.isFalse(wrapper.exists('DueDateSelector')); - assert.isTrue(wrapper.exists('ContentSelector')); - }); + // The three tests below only make sense while the due-date step is + // reachable. Restore them alongside the step, and put the due-date hop back + // into the test above. + // + // it('blocks the due-date step while the selector reports an invalid value', () => { + // fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; + // const wrapper = renderFilePicker(); + // + // selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint + // clickNext(wrapper); // -> due-date + // assert.isTrue(wrapper.exists('DueDateSelector')); + // + // // Whatever the reason — a half-entered value, or a complete one that is + // // no longer in the future — the selector shows it inline; the parent + // // only acts on the verdict. + // const handle = setDueDateValidity(wrapper, false); + // + // clickNext(wrapper); + // assert.isTrue(wrapper.exists('DueDateSelector')); + // assert.isFalse(wrapper.exists('ContentSelector')); + // assert.called(handle.validate); + // }); + // + // it('anchors the earliest selectable due date to the present', () => { + // fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; + // const wrapper = renderFilePicker(); + // + // selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint + // clickNext(wrapper); // -> due-date + // + // // The selector rejects anything below `min` (past dates, past times on + // // the current day); pinning `min` to "now" is the parent's half of the + // // "no due dates in the past" rule. + // const { min } = wrapper.find('DueDateSelector').first().props(); + // assert.isTrue(min <= dueDateFromNow(0)); + // assert.isTrue(min > dueDateFromNow(-1)); + // }); + // + // it('leaves the due-date step when the selector reports a valid value', () => { + // fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; + // const wrapper = renderFilePicker(); + // + // selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint + // clickNext(wrapper); // -> due-date + // + // setDueDateValidity(wrapper, true); + // setDueDate(wrapper, dueDateFromNow(7)); + // + // clickNext(wrapper); + // assert.isFalse(wrapper.exists('DueDateSelector')); + // assert.isTrue(wrapper.exists('ContentSelector')); + // }); it('does not offer a "Back" button on the first step', () => { fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; @@ -304,13 +307,9 @@ describe('FilePickerApp', () => { fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; const wrapper = renderFilePicker(); + // The due-date step this test used to walk back from is skipped for now. selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint - clickNext(wrapper); // -> due-date - assert.isTrue(wrapper.exists('DueDateSelector')); - - clickBack(wrapper); // -> checkpoint assert.isTrue(wrapper.exists('CheckpointSelector')); - assert.isFalse(wrapper.exists('DueDateSelector')); clickBack(wrapper); // -> assignment-type assert.isTrue(wrapper.exists('AssignmentTypeSelector')); @@ -328,9 +327,7 @@ describe('FilePickerApp', () => { selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint assert.equal(cardTitle(), 'Paced Social Annotation'); - clickNext(wrapper); // -> due-date - assert.equal(cardTitle(), 'Paced Social Annotation'); - + // The skipped due-date step shared this title. clickNext(wrapper); // -> regular flow assert.equal(cardTitle(), 'Assignment details'); }); @@ -342,9 +339,9 @@ describe('FilePickerApp', () => { // The mode-selection step itself offers no close button. assert.isNotOk(wrapper.find('CardHeader').prop('onClose')); + // Checked from the checkpoint step while the due-date step is skipped. selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint - clickNext(wrapper); // -> due-date - assert.isTrue(wrapper.exists('DueDateSelector')); + assert.isTrue(wrapper.exists('CheckpointSelector')); // The header exposes a close handler during the Paced sub-steps. const onClose = wrapper.find('CardHeader').prop('onClose'); @@ -352,7 +349,7 @@ describe('FilePickerApp', () => { interact(wrapper, () => onClose()); assert.isTrue(wrapper.exists('AssignmentTypeSelector')); - assert.isFalse(wrapper.exists('DueDateSelector')); + assert.isFalse(wrapper.exists('CheckpointSelector')); }); it('recomputes the branch when the type is changed after going back', () => { @@ -505,53 +502,57 @@ describe('FilePickerApp', () => { }); }); - it('sends the due date as a UTC datetime for "Hide & Reveal"', async () => { - fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; - const onSubmit = sinon.stub().callsFake(e => e.preventDefault()); - const wrapper = renderFilePicker({ onSubmit }); - - const clickNext = () => - interact(wrapper, () => { - wrapper - .find('Button[data-testid="workflow-next-button"]') - .props() - .onClick(); - }); - - // Walk the Hide & Reveal workflow, picking a future due date. The picker - // value is local wall-clock time; the backend receives it as UTC. - interact(wrapper, () => { - // Selecting the type advances straight to the checkpoint step. - wrapper - .find('AssignmentTypeSelector') - .props() - .onSelect('hide_and_reveal'); - }); - clickNext(); // -> due-date - const localDueDate = '2035-01-15T10:30'; - interact(wrapper, () => { - wrapper.find('DueDateSelector').first().props().onChange(localDueDate); - }); - clickNext(); // -> content selection - - selectContent(wrapper, 'https://example.com'); - - await waitFor(() => fakeAPICall.called); - assert.calledWith(fakeAPICall, { - authToken: 'DUMMY_AUTH_TOKEN', - path: deepLinkingAPIPath, - data: { - ...deepLinkingAPIData, - content: { type: 'url', url: 'https://example.com' }, - title: null, - group_set: null, - auto_grading_config: null, - assignment_gradable_max_points: null, - checkpoint_enabled: true, - due_date: new Date(localDueDate).toISOString(), - }, - }); - }); + // No due date can be picked while the step is skipped, so the payload + // always carries `due_date: null` (asserted by the tests above). Restore + // alongside the step. + // + // it('sends the due date as a UTC datetime for "Hide & Reveal"', async () => { + // fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; + // const onSubmit = sinon.stub().callsFake(e => e.preventDefault()); + // const wrapper = renderFilePicker({ onSubmit }); + // + // const clickNext = () => + // interact(wrapper, () => { + // wrapper + // .find('Button[data-testid="workflow-next-button"]') + // .props() + // .onClick(); + // }); + // + // // Walk the Hide & Reveal workflow, picking a future due date. The picker + // // value is local wall-clock time; the backend receives it as UTC. + // interact(wrapper, () => { + // // Selecting the type advances straight to the checkpoint step. + // wrapper + // .find('AssignmentTypeSelector') + // .props() + // .onSelect('hide_and_reveal'); + // }); + // clickNext(); // -> due-date + // const localDueDate = '2035-01-15T10:30'; + // interact(wrapper, () => { + // wrapper.find('DueDateSelector').first().props().onChange(localDueDate); + // }); + // clickNext(); // -> content selection + // + // selectContent(wrapper, 'https://example.com'); + // + // await waitFor(() => fakeAPICall.called); + // assert.calledWith(fakeAPICall, { + // authToken: 'DUMMY_AUTH_TOKEN', + // path: deepLinkingAPIPath, + // data: { + // ...deepLinkingAPIData, + // content: { type: 'url', url: 'https://example.com' }, + // title: null, + // group_set: null, + // auto_grading_config: null, + // assignment_gradable_max_points: null, + // checkpoint_enabled: true, + // due_date: new Date(localDueDate).toISOString(), + // }, + // }); + // }); it('fetches form field values via deep linking API on implicit form submission', async () => { // Enable title field, which is a field where the user could trigger an From 4589c8bd4adf97b9d42df38167d0a8ed4ee9d7e0 Mon Sep 17 00:00:00 2001 From: Karen Rasmussen Date: Mon, 10 Aug 2026 16:08:10 -0300 Subject: [PATCH 3/3] Fix --- lms/static/scripts/frontend_apps/components/FilePickerApp.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx b/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx index f9dd82921e..46825621e0 100644 --- a/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx +++ b/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx @@ -330,7 +330,7 @@ export default function FilePickerApp({ onSubmit }: FilePickerAppProps) { ) { return; } - // TEMPORARY, at Michael's request: the due-date step is skipped because the + // TEMPORARY: the due-date step is skipped because the // date it collects has no effect yet, and offering it suggests the // assignment does something it doesn't. Nothing else about the step was // removed — to turn it back on, restore the commented-out line below in