From a67da931992dddde10dee6cbde9ea314bee31633 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Sun, 27 Oct 2024 09:07:28 +0000 Subject: [PATCH 1/4] vault: Add smoke tests for 'On Completion Delete' See #3148, which I failed to spot when running the smoke tests previosuly. --- .../Smoke Testing the Tasks Plugin.md | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/resources/sample_vaults/Tasks-Demo/Manual Testing/Smoke Testing the Tasks Plugin.md b/resources/sample_vaults/Tasks-Demo/Manual Testing/Smoke Testing the Tasks Plugin.md index f72cb2e3c7..a213926997 100644 --- a/resources/sample_vaults/Tasks-Demo/Manual Testing/Smoke Testing the Tasks Plugin.md +++ b/resources/sample_vaults/Tasks-Demo/Manual Testing/Smoke Testing the Tasks Plugin.md @@ -58,25 +58,27 @@ Work through all the tasks below, until zero tasks remain in this query: ## The Smoke Tests -### Completion of tasks +### Toggling tasks + +#### Completion of tasks - [ ] #task Mark this task complete in **Source view** using **Tasks: Toggle task done** command - [ ] #task Mark this task complete by clicking on it in **Reading view** -- [ ] #task Mark this task complete by clicking on it in **Live Preview** +- [ ] #task Mark this task complete by clicking on it in **Live Preview** - ==ensure the checkbox is redrawn correctly== - [ ] #task **check**: Checked all above methods for **completing tasks** - and they worked -### Un-completion of tasks +#### Un-completion of tasks * [x] #task Mark this task not complete in **Source view** using **Tasks: Toggle task done** command ✅ 2022-07-05 * [x] #task Mark this task not complete by clicking on it in **Reading view** ✅ 2022-07-05 -* [x] #task Mark this task not complete by clicking on it in **Live Preview** ✅ 2022-07-05 +* [x] #task Mark this task not complete by clicking on it in **Live Preview** - ==ensure the checkbox is redrawn correctly== ✅ 2022-07-05 * [ ] #task **check**: Checked all above methods for **un-completing tasks** - and they worked -### Recurring Tasks +#### Recurring Tasks Confirm that when a recurring task is completed, a new task is created, all the date fields are incremented, and the indentation is unchanged. @@ -84,12 +86,32 @@ Confirm that when a recurring task is completed, a new task is created, all the > > - [ ] #task Complete this recurring task in **Source view** using **Tasks: Toggle task done** command 🔁 every day 🛫 2022-02-17 ⏳ 2022-02-18 📅 2022-02-19 > -> > - [ ] #task Complete this recurring task in **Reading view**🔁 every day 🛫 2022-02-17 ⏳ 2022-02-18 📅 2022-02-19 +> > - [ ] #task Complete this recurring task in **Reading view** 🔁 every day 🛫 2022-02-17 ⏳ 2022-02-18 📅 2022-02-19 -- [ ] #task Complete this recurring task in **Live Preview**🔁 every day 🛫 2022-02-17 ⏳ 2022-02-18 📅 2022-02-19 +- [ ] #task Complete this recurring task in **Live Preview** - ==ensure the checkbox is redrawn correctly== 🔁 every day 🛫 2022-02-17 ⏳ 2022-02-18 📅 2022-02-19 > - [ ] #task **check**: Checked all above steps for **recurring tasks** worked +### On Completion + +#### On Completion Delete: Non-recurring tasks + +Confirm that the **task line is deleted**. + +- [ ] #task Complete this auto-deleting non-recurring task in **Source view** using **Tasks: Toggle task done** command 🏁 delete 📅 2024-10-27 +- [ ] #task Complete this auto-deleting non-recurring task in **Reading view** 🏁 delete 📅 2024-10-27 +- [ ] #task Complete this auto-deleting non-recurring task in **Live Preview** 🏁 delete 📅 2024-10-27 +- [ ] #task **check**: Checked all above steps for **auto-deleting non-recurring tasks** worked + +#### On Completion Delete: Recurring Tasks + +Confirm that when an auto-deleting recurring task is completed, a **new task is created replacing the old task**, and the checkbox remains not-done. + +- [ ] #task Complete this auto-deleting recurring task in **Source view** using **Tasks: Toggle task done** command 🔁 every day 🏁 delete 📅 2024-10-27 +- [ ] #task Complete this auto-deleting recurring task in **Reading view** 🔁 every day 🏁 delete 📅 2024-10-27 +- [ ] #task Complete this auto-deleting recurring task in **Live Preview** - ==ensure the checkbox is redrawn correctly== 🔁 every day 🏁 delete 📅 2024-10-27 +- [ ] #task **check**: Checked all above steps for **auto-deleting recurring tasks** worked + ### Rendering of Task Blocks Steps to do: From 019361bb13824405676f0f0112bd6e43cca270c2 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Sun, 27 Oct 2024 09:52:02 +0000 Subject: [PATCH 2/4] Revert "fix: 'on completion' delete now renders correctly in Live Preview" This reverts commit a76abe30c5354dc8c6fa73196c3761b2801aa390. --- src/Obsidian/LivePreviewExtension.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/Obsidian/LivePreviewExtension.ts b/src/Obsidian/LivePreviewExtension.ts index 6048d352f5..a75d95d7ba 100644 --- a/src/Obsidian/LivePreviewExtension.ts +++ b/src/Obsidian/LivePreviewExtension.ts @@ -90,6 +90,19 @@ class LivePreviewExtension implements PluginValue { }); this.view.dispatch(transaction); + // Dirty workaround. + // While the code in this method properly updates the `checked` state + // of the target checkbox, some Obsidian internals revert the state. + // This means that the checkbox would remain in its original `checked` + // state (`true` or `false`), even though the underlying document + // updates correctly. + // As a "fix", we set the checkbox's `checked` state *again* after a + // timeout to revert Obsidian's wrongful reversal. + const desiredCheckedStatus = target.checked; + setTimeout(() => { + target.checked = desiredCheckedStatus; + }, 1); + return true; } } From 989f11bd1d691fc863f45fafa1e33b13ddb58aac Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Sun, 27 Oct 2024 10:53:06 +0000 Subject: [PATCH 3/4] fix: Correct rendering of checkboxes for non-recurring tasks in Live Preview Fixes #3148 whilst retaining the fix for #3043. --- src/Obsidian/LivePreviewExtension.ts | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Obsidian/LivePreviewExtension.ts b/src/Obsidian/LivePreviewExtension.ts index a75d95d7ba..7fab4cf692 100644 --- a/src/Obsidian/LivePreviewExtension.ts +++ b/src/Obsidian/LivePreviewExtension.ts @@ -96,12 +96,21 @@ class LivePreviewExtension implements PluginValue { // This means that the checkbox would remain in its original `checked` // state (`true` or `false`), even though the underlying document // updates correctly. - // As a "fix", we set the checkbox's `checked` state *again* after a - // timeout to revert Obsidian's wrongful reversal. - const desiredCheckedStatus = target.checked; - setTimeout(() => { - target.checked = desiredCheckedStatus; - }, 1); + // As a "fix", we set the checkbox's `checked` state *explicitly* after a + // timeout in case we need to revert Obsidian's possibly wrongful reversal. + if (toggled.length === 1) { + // The smoke tests show the workaround is only needed when the event replaces + // a single task line. + // (When one task line becomes two because of recurrence, both the + // edited task lines are rendered correctly by this code) + // Since the advent of 'on completion: delete', we cannot rely on the + // event target's opinion of the new status, as that facility means + // that the new status *may* be different from that in the event. + const desiredCheckedStatus = toggled[0].status.symbol !== ' '; + setTimeout(() => { + target.checked = desiredCheckedStatus; + }, 1); + } return true; } From 86bffd5b47e58579beffa81f4335955d017753b4 Mon Sep 17 00:00:00 2001 From: Clare Macrae Date: Sun, 27 Oct 2024 10:55:14 +0000 Subject: [PATCH 4/4] refactor: . Extract variable needToForceCheckedProperty for clarity --- src/Obsidian/LivePreviewExtension.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Obsidian/LivePreviewExtension.ts b/src/Obsidian/LivePreviewExtension.ts index 7fab4cf692..5ed6d50952 100644 --- a/src/Obsidian/LivePreviewExtension.ts +++ b/src/Obsidian/LivePreviewExtension.ts @@ -98,7 +98,8 @@ class LivePreviewExtension implements PluginValue { // updates correctly. // As a "fix", we set the checkbox's `checked` state *explicitly* after a // timeout in case we need to revert Obsidian's possibly wrongful reversal. - if (toggled.length === 1) { + const needToForceCheckedProperty = toggled.length === 1; + if (needToForceCheckedProperty) { // The smoke tests show the workaround is only needed when the event replaces // a single task line. // (When one task line becomes two because of recurrence, both the