fix: warn when a pending edit is not accepted by JohnMcLear · Pull Request #7540 · ether/etherpad

JohnMcLear · 2026-04-17T16:41:22Z

Summary

show a gritter warning only if the pad disconnects while a local commit is still awaiting acceptance
clear the pending-commit marker as soon as ACCEPT_COMMIT arrives
add localized copy for the unsaved edit warning

Testing

pnpm --filter ep_etherpad-lite run ts-check

~20 seconds to get this error tho... might be too long for good UX

Show a gritter warning only when the pad disconnects while a local commit is still awaiting acceptance, leaving normal editing UI unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

qodo-free-for-open-source-projects · 2026-04-17T16:41:37Z

Review Summary by Qodo

Warn when pending edit is not accepted on disconnect

🐞 Bug fix

Walkthroughs

Description

• Clear pending commit marker when ACCEPT_COMMIT arrives
• Show gritter warning only on disconnect with unaccepted edits
• Add localized strings for unsaved edit warning
• Export method to check for unaccepted commits

Diagram

flowchart LR
  A["Commit sent<br/>stateMessage set"] -->|ACCEPT_COMMIT| B["Clear stateMessage<br/>setStateIdle"]
  C["Pad disconnects"] -->|hasUnacceptedCommit| D{stateMessage<br/>!= null?}
  D -->|Yes| E["Show gritter warning<br/>Unsaved edit"]
  D -->|No| F["Normal disconnect"]

File Changes

1. src/static/js/collab_client.ts 🐞 Bug fix +2/-0

Clear pending commit and expose unaccepted commit check

• Clear stateMessage to null when ACCEPT_COMMIT is received
• Export new hasUnacceptedCommit() method to check if pending commit exists
• Allows external code to detect unaccepted commits

src/static/js/collab_client.ts

2. src/static/js/pad.ts 🐞 Bug fix +9/-0

Show warning on disconnect with unaccepted edits

• Add showUnacceptedCommitWarning() method to display gritter notification
• Call warning method in handleChannelStateChange() when disconnecting with unaccepted commits
• Uses sticky gritter with disconnected and unsaved-warning CSS classes

src/static/js/pad.ts

3. src/locales/en.json 📝 Documentation +2/-0

Add localized strings for unsaved edit warning

• Add localized title: "Unsaved edit"
• Add localized text: "Your recent edit is still not saved. Reconnect and try again."
• Provides user-facing messaging for the warning notification

src/locales/en.json

qodo-free-for-open-source-projects · 2026-04-17T16:41:38Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (1)

1. Pending edits only warned on disconnect 📎 Requirement gap ☼ Reliability

Description

The new UX warning for unaccepted commits is only shown when the channel enters DISCONNECTED, so
users can still have locally-applied edits pending acceptance with no in-UI indication during the
pending state. This fails the requirement to indicate the pending state itself (and clear/update
once accepted).

Code

src/static/js/pad.ts[703]

+      if (pad.collabClient.hasUnacceptedCommit()) pad.showUnacceptedCommitWarning();

Evidence
PR Compliance ID 1 requires a clear indication while a changeset is pending (before ACCEPT_COMMIT)
and removed/updated once accepted. The added code only triggers the warning inside the
DISCONNECTED branch (handleChannelStateChange), even though pending-state detection exists via
hasUnacceptedCommit(), meaning there is still no UI indication during normal connected pending
state.
UI indicates when local edits are pending server acceptance (no ACCEPT_COMMIT yet)
src/static/js/pad.ts[703-703]
src/static/js/collab_client.ts[492-492]
src/static/js/collab_client.ts[142-145]

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 UI warning for unaccepted commits is only displayed upon disconnect, not throughout the entire pending-acceptance window. Compliance requires a clear indication whenever a commit is pending (until `ACCEPT_COMMIT` clears it).

## Issue Context
A pending commit is tracked via `stateMessage` and exposed as `hasUnacceptedCommit()`, and `stateMessage` is cleared in `acceptCommit()`. However, the UI only calls `showUnacceptedCommitWarning()` inside the `DISCONNECTED` channel state, so users may still believe edits are saved while connected.

## Fix Focus Areas
- src/static/js/pad.ts[658-665]
- src/static/js/pad.ts[703-703]
- src/static/js/collab_client.ts[142-151]
- src/static/js/collab_client.ts[492-500]

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

2. No test for unaccepted warning 📘 Rule violation ☼ Reliability

Description

This bug fix changes client behavior around pending commits and disconnect warnings but does not
include any regression test in the change set. Without an automated test, the issue can reappear
undetected if the fix is reverted or refactored.

Code

src/static/js/pad.ts[R658-665]

+  showUnacceptedCommitWarning: () => {
+    $.gritter.add({
+      title: html10n.get('pad.gritter.unacceptedCommit.title'),
+      text: html10n.get('pad.gritter.unacceptedCommit.text'),
+      sticky: true,
+      class_name: 'disconnected unsaved-warning',
+    });
+  },

Evidence
PR Compliance ID 2 requires a regression test to accompany bug fixes. The PR adds new runtime
behavior (showUnacceptedCommitWarning() and the disconnect-time check) but there are no
corresponding test additions/changes shown alongside these modifications.
src/static/js/pad.ts[658-665]
src/static/js/pad.ts[703-703]
src/static/js/collab_client.ts[142-145]
Best Practice: Repository guidelines

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 PR changes client behavior for pending commits/disconnect warnings but does not add a regression test that would fail without the fix.

## Issue Context
A suitable regression test should assert at least one of:
- `stateMessage`/pending marker is cleared when `ACCEPT_COMMIT` is processed (so `hasUnacceptedCommit()` flips to `false`).
- The warning UI is triggered when a disconnect occurs while a commit is pending.

## Fix Focus Areas
- src/static/js/collab_client.ts[142-151]
- src/static/js/collab_client.ts[492-500]
- src/static/js/pad.ts[658-665]
- src/static/js/pad.ts[703-703]

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

3. Sticky warning not cleared 🐞 Bug ≡ Correctness

Description

pad.showUnacceptedCommitWarning() creates a sticky gritter notification, but there is no code path
that removes it when the client reconnects or the pending commit becomes accepted, leaving users
with a persistent false 'Unsaved edit' warning.

Code

src/static/js/pad.ts[R658-665]

+  showUnacceptedCommitWarning: () => {
+    $.gritter.add({
+      title: html10n.get('pad.gritter.unacceptedCommit.title'),
+      text: html10n.get('pad.gritter.unacceptedCommit.text'),
+      sticky: true,
+      class_name: 'disconnected unsaved-warning',
+    });
+  },

Evidence

The new warning is explicitly created as a sticky gritter notification, and Gritter only schedules
auto-fade when sticky is false, so it will remain until manually closed.
pad.handleChannelStateChange() shows the warning on DISCONNECTED but does not remove it on
CONNECTED/RECONNECTING, so the UI can remain in a warning state even after the underlying
pending-commit marker is cleared. The codebase already uses DOM selection +
$.gritter.remove(this.id) to clear specific notifications elsewhere, demonstrating a supported
removal approach.

src/static/js/pad.ts[658-704]
src/static/js/vendors/gritter.ts[116-187]
src/static/js/chat.ts[37-45]

Agent prompt

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

### Issue description
A sticky gritter notification (`unsaved-warning`) is added on DISCONNECTED when there is an unaccepted commit, but it is never removed on reconnect or after the commit is accepted. This can leave users with a persistent and eventually incorrect warning.

### Issue Context
- `showUnacceptedCommitWarning()` sets `sticky: true`, so the notification will not auto-expire.
- `handleChannelStateChange('CONNECTED')` does not remove any existing `unsaved-warning` notifications.
- Other code (chat) removes specific gritter notifications by selecting DOM nodes and calling `$.gritter.remove(this.id)`.

### Fix Focus Areas
- src/static/js/pad.ts[658-704]

### Implementation notes
- Before adding the warning, remove any existing `.gritter-item.unsaved-warning` items to avoid duplicates.
- Remove `.gritter-item.unsaved-warning` when transitioning to `CONNECTED` (and optionally when transitioning to `RECONNECTING`).
- Example removal pattern:
 - `$('.gritter-item.unsaved-warning').each(function () { $.gritter.remove(this.id); });`
- Optionally store the created gritter DOM id and remove it directly for less DOM querying.

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

ⓘ The new review experience is currently in Beta. Learn more

qodo-free-for-open-source-projects · 2026-04-17T16:46:02Z

      padimpexp.disable();

      padconnectionstatus.disconnected(message);
+      if (pad.collabClient.hasUnacceptedCommit()) pad.showUnacceptedCommitWarning();


1. Pending edits only warned on disconnect 📎 Requirement gap ☼ Reliability

The new UX warning for unaccepted commits is only shown when the channel enters DISCONNECTED, so users can still have locally-applied edits pending acceptance with no in-UI indication during the pending state. This fails the requirement to indicate the pending state itself (and clear/update once accepted).

Agent Prompt

## Issue description The UI warning for unaccepted commits is only displayed upon disconnect, not throughout the entire pending-acceptance window. Compliance requires a clear indication whenever a commit is pending (until `ACCEPT_COMMIT` clears it). ## Issue Context A pending commit is tracked via `stateMessage` and exposed as `hasUnacceptedCommit()`, and `stateMessage` is cleared in `acceptCommit()`. However, the UI only calls `showUnacceptedCommitWarning()` inside the `DISCONNECTED` channel state, so users may still believe edits are saved while connected. ## Fix Focus Areas - src/static/js/pad.ts[658-665] - src/static/js/pad.ts[703-703] - src/static/js/collab_client.ts[142-151] - src/static/js/collab_client.ts[492-500]

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

qodo-free-for-open-source-projects · 2026-04-17T16:46:02Z

+  showUnacceptedCommitWarning: () => {
+    $.gritter.add({
+      title: html10n.get('pad.gritter.unacceptedCommit.title'),
+      text: html10n.get('pad.gritter.unacceptedCommit.text'),
+      sticky: true,
+      class_name: 'disconnected unsaved-warning',
+    });
+  },


2. No test for unaccepted warning 📘 Rule violation ☼ Reliability

This bug fix changes client behavior around pending commits and disconnect warnings but does not include any regression test in the change set. Without an automated test, the issue can reappear undetected if the fix is reverted or refactored.

Agent Prompt

## Issue description The PR changes client behavior for pending commits/disconnect warnings but does not add a regression test that would fail without the fix. ## Issue Context A suitable regression test should assert at least one of: - `stateMessage`/pending marker is cleared when `ACCEPT_COMMIT` is processed (so `hasUnacceptedCommit()` flips to `false`). - The warning UI is triggered when a disconnect occurs while a commit is pending. ## Fix Focus Areas - src/static/js/collab_client.ts[142-151] - src/static/js/collab_client.ts[492-500] - src/static/js/pad.ts[658-665] - src/static/js/pad.ts[703-703]

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

fix: warn when a pending edit is not accepted

2baedb6

Show a gritter warning only when the pad disconnects while a local commit is still awaiting acceptance, leaving normal editing UI unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

qodo-free-for-open-source-projects bot reviewed Apr 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: warn when a pending edit is not accepted#7540

fix: warn when a pending edit is not accepted#7540
JohnMcLear wants to merge 1 commit intoether:developfrom
JohnMcLear:fix-unaccepted-commit-warning

JohnMcLear commented Apr 17, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects bot commented Apr 17, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Apr 17, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects bot Apr 17, 2026

Uh oh!

qodo-free-for-open-source-projects bot Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

JohnMcLear commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

qodo-free-for-open-source-projects bot commented Apr 17, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-free-for-open-source-projects bot commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

qodo-free-for-open-source-projects bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-free-for-open-source-projects bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

JohnMcLear commented Apr 17, 2026 •

edited

Loading

qodo-free-for-open-source-projects bot commented Apr 17, 2026 •

edited

Loading