fix: delete a sticky note on close instead of only hiding it - #345
Open
itzmekhokan wants to merge 3 commits into
Open
fix: delete a sticky note on close instead of only hiding it#345itzmekhokan wants to merge 3 commits into
itzmekhokan wants to merge 3 commits into
Conversation
The note's title-bar × was labelled "Hide sticky note" and only called layer.forget() — an in-memory eviction. The backing wp_guideline post survived, so the note reappeared on the next heartbeat tick, reload, or any desktop re-render. With no other removal affordance, notes could never actually be deleted (GH#344). Make the × delete the note, following the same soft-trash + Undo pattern the desktop-files layer already uses: - rest.ts: add deleteStickyNote() (REST trash, not force, so it stays recoverable) and restoreStickyNote() (untrash by flipping status back to private). fetchStickyNotes only queries status=private, so a trashed note never re-hydrates. - layer.ts: the × now trashes the note with optimistic eviction and an Undo toast; relabel it "Delete sticky note". A `dismissedIds` guard drops any in-flight heartbeat/reload echo for a just-deleted id so it can't resurrect the note before the trash propagates. - A note created and closed inside the save debounce is persisted first (save() now hands callers the in-flight promise) so there's always a server row to trash — closing the create-then-delete race that caused the intermittent resurrection. Adds vitest coverage for the trash path and the heartbeat-echo guard. Verified in wp-env: deleting a note trashes the post and it no longer returns after a reload; Undo restores it.
Collaborator
|
Hey Thanks for your contribution! I'll ask the person who made the Sticky Notes, not sure if he really intended to hide for the session or close them forever. @artpi? |
renderToast() flipped the enter state ("in") inside a
requestAnimationFrame so the CSS fade runs after first paint. rAF is
paused while the tab is hidden/backgrounded, so a toast raised then
stayed at opacity:0 — and could hit its dismiss timer before ever
showing. Pair the rAF with a setTimeout fallback (which still fires in
a hidden tab); the enter callback is idempotent, so whichever runs
first wins and the other is a no-op.
Collaborator
|
I'll reach Artur internally 😆 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #344.
The sticky-note title-bar × was labelled "Hide sticky note" and only called
layer.forget()— an in-memory eviction. The backingwp_guidelinepost was never touched, so the note reappeared on the next heartbeat tick, page reload, or any desktop re-render. Since the × was the only removal affordance, a note could never actually be deleted.Fix
Make the × delete the note, following the same soft-trash + Undo pattern the desktop-files layer (
trashPlacementWithUndo) already uses:rest.ts— adddeleteStickyNote()(REST trash, notforce, so it stays recoverable) andrestoreStickyNote()(untrash by flipping status back toprivate).fetchStickyNotesonly queriesstatus: 'private', so a trashed note never re-hydrates onto the desktop.layer.ts— the × now trashes the note with optimistic eviction and an Undo toast, and is relabelled "Delete sticky note". AdismissedIdsguard drops any in-flight heartbeat/reload echo for a just-deleted id so it can't resurrect the note before the trash propagates.save()now hands callers its in-flight promise) so there's always a server row to trash — closing the create-then-delete race behind the intermittent resurrection.No behaviour change for the "empty, never-saved note" case (still dropped locally with no round-trip).
Testing
npm run typecheck,npm run lint— cleannpm run test:js— 1610 pass (adds 2 tests: the × trashes + evicts the note, and a stale heartbeat echo can't re-add a deleted note)wp-env:wp_guidelinepost moves totrash.0notes and the post stayingtrash.restoreStickyNoteflips the post back toprivateand re-adds the note.