Skip to content

Commit 2f0b5b0

Browse files
JohnMcLearCopilotclaude
authored
fix: warn when a pending edit is not accepted (#7540)
* fix: warn when a pending edit is not accepted 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> * test: cover unaccepted-commit warning path Addresses Qodo review: adds regression coverage for the two contract changes this PR introduces — acceptCommit() must clear the pending marker so hasUnacceptedCommit() returns false after a server ACK, and the disconnect handler must surface the unsaved-edit gritter when a commit is still pending. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4137109 commit 2f0b5b0

4 files changed

Lines changed: 52 additions & 0 deletions

File tree

src/locales/en.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@
151151
"pad.modals.disconnected": "You have been disconnected.",
152152
"pad.modals.disconnected.explanation": "The connection to the server was lost",
153153
"pad.modals.disconnected.cause": "The server may be unavailable. Please notify the service administrator if this continues to happen.",
154+
"pad.gritter.unacceptedCommit.title": "Unsaved edit",
155+
"pad.gritter.unacceptedCommit.text": "Your recent edit is still not saved. Reconnect and try again.",
154156

155157
"pad.share": "Share this pad",
156158
"pad.share.readonly": "Read only",

src/static/js/collab_client.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad)
141141

142142
const acceptCommit = () => {
143143
editor.applyPreparedChangesetToBase();
144+
stateMessage = null;
144145
setStateIdle();
145146
try {
146147
callbacks.onInternalAction('commitAcceptedByServer');
@@ -488,6 +489,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad)
488489
sendMessage,
489490
getCurrentRevisionNumber,
490491
getMissedChanges,
492+
hasUnacceptedCommit: () => stateMessage != null,
491493
callWhenNotCommitting,
492494
addHistoricalAuthors: tellAceAboutHistoricalAuthors,
493495
setChannelState,

src/static/js/pad.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,14 @@ const pad = {
655655
pad.handleOptionsChange(opts);
656656
}
657657
},
658+
showUnacceptedCommitWarning: () => {
659+
$.gritter.add({
660+
title: html10n.get('pad.gritter.unacceptedCommit.title'),
661+
text: html10n.get('pad.gritter.unacceptedCommit.text'),
662+
sticky: true,
663+
class_name: 'disconnected unsaved-warning',
664+
});
665+
},
658666
handleChannelStateChange: (newState, message) => {
659667
const oldFullyConnected = !!padconnectionstatus.isFullyConnected();
660668
const wasConnecting = (padconnectionstatus.getStatus().what === 'connecting');
@@ -692,6 +700,7 @@ const pad = {
692700
padimpexp.disable();
693701

694702
padconnectionstatus.disconnected(message);
703+
if (pad.collabClient.hasUnacceptedCommit()) pad.showUnacceptedCommitWarning();
695704
}
696705
const newFullyConnected = !!padconnectionstatus.isFullyConnected();
697706
if (newFullyConnected !== oldFullyConnected) {
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import {expect, test} from '@playwright/test';
2+
import {clearPadContent, goToNewPad, writeToPad} from '../helper/padHelper';
3+
4+
test.describe('unaccepted commit warning', () => {
5+
test('hasUnacceptedCommit clears once the server acknowledges the commit',
6+
async ({page}) => {
7+
await goToNewPad(page);
8+
await clearPadContent(page);
9+
await writeToPad(page, 'trigger a commit');
10+
11+
// Wait for the commit to round-trip. The fix clears the pending marker inside
12+
// acceptCommit(); without it the boolean stays true indefinitely.
13+
await expect.poll(async () => await page.evaluate(() =>
14+
(window as any).pad?.collabClient?.hasUnacceptedCommit?.() ?? null,
15+
), {timeout: 10000}).toBe(false);
16+
});
17+
18+
test('disconnect with a pending commit surfaces the unsaved-edit gritter',
19+
async ({page}) => {
20+
await goToNewPad(page);
21+
await page.waitForFunction(() => (window as any).pad?.collabClient != null);
22+
23+
await page.evaluate(() => {
24+
const p: any = (window as any).pad;
25+
// Force the pending-commit predicate to true and simulate a disconnect so
26+
// the warning code path executes deterministically.
27+
p.collabClient.hasUnacceptedCommit = () => true;
28+
p.handleChannelStateChange('DISCONNECTED', {
29+
type: 'disconnect',
30+
explanation: 'test',
31+
cause: 'test',
32+
forIE: false,
33+
canRetry: false,
34+
});
35+
});
36+
37+
await expect(page.locator('.unsaved-warning').first()).toBeVisible();
38+
});
39+
});

0 commit comments

Comments
 (0)