Skip to content

Show a toast after a manual save - #119

Merged
koppor merged 4 commits into
mainfrom
save-toast
Oct 26, 2026
Merged

koppor merged 4 commits into
mainfrom
save-toast

Conversation

@koppor

@koppor koppor commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

With autosave off, nothing told the user whether clicking Save or pressing Ctrl+S did anything. A short toast now confirms a successful save. With autosave on, saves stay silent, since one would otherwise pop up after every change.

Fixes #38

Toast after saving

How to test (JabRef with HTTP server enabled, or jabsrv):

  1. Switch "Autosave" off, click "Save" or press Ctrl+S: a toast appears bottom right and disappears after a few seconds.
  2. Switch "Autosave" on, change a node or click "Save": no toast.
  3. Stop JabRef, click "Save": the button turns red as before, no toast.

Tested the first two steps in the Electron build against a stub of the HTTP server; the failed-save path is unchanged.

🤖 Generated with Claude Code

With autosave off, nothing told the user whether clicking Save or
pressing Ctrl+S did anything. A short toast now confirms a successful
save; with autosave on, saves are silent as before.

Fixes #38

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@koppor
koppor marked this pull request as ready for review October 26, 2026 00:12
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Confirm successful saves when autosave is off

✨ Enhancement 📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Show a two-second confirmation toast after a successful save when autosave is off.
• Keep saves silent when autosave is on and retain the red button for failed saves.
• Document the confirmation in the changelog.
Diagram

graph TD
  Save["Save events"] --> Queue["Queued saves"] --> Server["JabRef HTTP"] --> Success{"Save successful?"} -->|Yes| Autosave{"Autosave off?"} -->|Yes| Toast["Success toast"]
  Success -->|No| Failure["Red save button"]
  Autosave -->|No| Silent["No toast"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Capture notification eligibility when queuing
  • ➕ Prevents an in-flight autosave from showing a toast if autosave is switched off before it completes.
  • ➖ Requires passing notification intent through the queued-save path instead of reading the current toggle.

Recommendation: The existing save path and Bootstrap toast are appropriate. Consider capturing toast eligibility when each save is queued if switching autosave during an in-flight request must not produce a misleading confirmation.

Files changed (3) +18 / -1

Enhancement (2) +17 / -1
index.htmlAdd accessible save-confirmation toast +11/-0

Add accessible save-confirmation toast

• Adds a bottom-right Bootstrap success toast with a close button and a two-second delay. The toast announces that the mind map was saved to JabRef.

index.html

main.jsShow toast after eligible successful saves +6/-1

Show toast after eligible successful saves

• Imports Bootstrap Toast and shows the confirmation after a successful queued save when autosave is off. Existing failed-save button styling remains in place.

src/main.js

Documentation (1) +1 / -0
CHANGELOG.mdDocument manual-save confirmation +1/-0

Document manual-save confirmation

• Adds an Unreleased entry describing the success toast when autosave is off and links issue #38.

CHANGELOG.md

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Users get the wrong save confirmation ✓ Resolved
Description
The toast guard reads autosaveCheck.checked only after httpClient.saveMap() completes, rather
than recording whether the queued save was manual or automatic. Toggling Autosave while a request is
pending can therefore hide a manual-save confirmation or display one for an autosave save.
Code

src/main.js[R302-303]

+        if (!hasFailed && !autosaveCheck.checked) {
+            Toast.getOrCreateInstance(savedToast).show();
Evidence
saveMap waits for the HTTP request before evaluating the checkbox, while both manual and autosave
paths enqueue the same function. Autosave is enabled by default, and the checkbox can be changed
independently while the request is pending, so the completion-time state does not reliably represent
the operation's origin.

src/main.js[294-305]
src/main.js[311-335]
index.html[95-96]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The success toast decision is made after the asynchronous save finishes by reading the current Autosave checkbox. If the user changes that checkbox while a save is in flight, a manual save can lose its toast or an autosave operation can show one.
### Fix Focus Areas
- src/main.js[294-305]
- src/main.js[311-335]
### Recommended Fix
Pass an explicit notification intent into `saveMap` when each operation is queued. Manual button and keyboard saves should request a toast, while autosave-triggered calls and the save performed when enabling Autosave should not; use that captured value after the request succeeds instead of rereading `autosaveCheck.checked`. Add a test covering a checkbox change during an in-flight save.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. Changelog uses a bare issue reference 📘 Rule violation ⚙ Maintainability
Description
The new changelog entry ends with [#38] instead of a full issue URL. Anyone following the
reference must rely on the bare number rather than an unambiguous link, which makes the new comment
inconsistent with the checklist's documentation requirement.
Code

CHANGELOG.md[17]

+- We added a toast confirming a successful save when autosave is off. [#38](https://github.com/JabRef/jabmap/issues/38)
Evidence
The changed entry introduces the bare issue reference [#38], which violates the requirement that
new issue or PR references use full URLs.

AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments
CHANGELOG.md[17-17]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new changelog entry uses the bare issue reference `[#38]`, while new issue and PR references must use full URLs.
## Fix Focus Areas
- CHANGELOG.md[17-17]
## Recommended Fix
Replace `[#38]` with a Markdown link to `https://github.com/JabRef/jabmap/issues/38`, matching the full-URL style required for new references.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Toast markup has an obvious comment ✓ Resolved
Description
The added `` comment only labels the immediately following toast markup. It adds no rationale and
leaves future readers with a redundant description of an obvious element instead of explaining a
non-obvious decision.
Code

index.html[311]

+  <!--Confirmation of a manual save-->
Evidence
The changed line is an explanatory label for the immediately following toast markup and provides no
non-obvious rationale, contrary to the comment-quality requirement.

AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments: AGENTS.md: Write Purposeful Code Comments
index.html[311-311]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new HTML comment merely describes the toast element below it and does not explain a non-obvious implementation decision.
## Fix Focus Areas
- index.html[311-311]
## Recommended Fix
Remove the `<!--Confirmation of a manual save-->` comment, or replace it with a comment that explains a genuinely non-obvious reason for the markup if one exists.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread CHANGELOG.md
- We added "Show in JabRef" (<kbd>Ctrl</kbd> + <kbd>J</kbd>) to select a BibTeX node's entry in JabRef. [#46](https://github.com/JabRef/jabmap/issues/46)
- We added this changelog. It is checked by [heylogs](https://github.com/nbbrd/heylogs) in CI. [#103](https://github.com/JabRef/jabmap/pull/103)
- We added autosave: after a mind map was opened, every change, undo and redo is saved to JabRef immediately. The "Autosave" toggle in the toolbar switches it off. A failed save turns the "Save" button red. [#36](https://github.com/JabRef/jabmap/issues/36)
- We added a toast confirming a successful save when autosave is off. [#38](https://github.com/JabRef/jabmap/issues/38)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Informational

2. Changelog uses a bare issue reference 📘 Rule violation ⚙ Maintainability

The new changelog entry ends with [#38] instead of a full issue URL. Anyone following the
reference must rely on the bare number rather than an unambiguous link, which makes the new comment
inconsistent with the checklist's documentation requirement.
Agent Prompt
## Issue description
The new changelog entry uses the bare issue reference `[#38]`, while new issue and PR references must use full URLs.

## Fix Focus Areas
- CHANGELOG.md[17-17]

## Recommended Fix
Replace `[#38]` with a Markdown link to `https://github.com/JabRef/jabmap/issues/38`, matching the full-URL style required for new references.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread index.html Resolved
Comment thread src/main.js Resolved
koppor and others added 3 commits October 26, 2026 01:14
Toggling autosave while a save was in flight could show or hide the
toast for the wrong save. Also reuse the toast that landed on main.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@koppor
koppor merged commit d88825e into main Oct 26, 2026
2 checks passed
@koppor
koppor deleted the save-toast branch October 26, 2026 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add notification on save

1 participant