Skip to content

Commit a790da1

Browse files
committed
fix UI diff scroll-sync-issue and not fetching historical revision on thing selection
1 parent 7073a7e commit a790da1

3 files changed

Lines changed: 89 additions & 3 deletions

File tree

ui/modules/things/subDiff.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export function SubDiff(targetTab, title: string) {
2828
let pendingLeft: string | null = null;
2929
let pendingRight: string | null = null;
3030
let onActivatedCallback: ((link: HTMLAnchorElement) => void) | null = null;
31+
let scrollSyncing = false;
3132

3233
return { ready, update, getTabLink, setOnActivated, destroy: destroyDiff };
3334

@@ -77,8 +78,12 @@ export function SubDiff(targetTab, title: string) {
7778

7879
if (diffInstance) {
7980
const editors = diffInstance.getEditors();
81+
scrollSyncing = true;
8082
editors.left.setValue(left, -1);
8183
editors.right.setValue(right, -1);
84+
editors.left.getSession().setScrollTop(0);
85+
editors.right.getSession().setScrollTop(0);
86+
scrollSyncing = false;
8287
diffInstance.diff();
8388
} else {
8489
diffInstance = new AceDiff({
@@ -87,7 +92,7 @@ export function SubDiff(targetTab, title: string) {
8792
mode: 'ace/mode/json',
8893
theme: null,
8994
diffGranularity: 'specific',
90-
lockScrolling: true,
95+
lockScrolling: false,
9196
showDiffs: true,
9297
showConnectors: true,
9398
charDiffs: true,
@@ -103,14 +108,45 @@ export function SubDiff(targetTab, title: string) {
103108
copyLinkEnabled: false,
104109
},
105110
});
111+
setupScrollSync();
106112
}
107113
}
108114

115+
function setupScrollSync() {
116+
if (!diffInstance) return;
117+
const editors = diffInstance.getEditors();
118+
119+
const sync = (source, target) => {
120+
if (scrollSyncing) return;
121+
scrollSyncing = true;
122+
123+
const srcSession = source.getSession();
124+
const tgtSession = target.getSession();
125+
const srcTop = srcSession.getScrollTop();
126+
const lineHeight = source.renderer.lineHeight || 16;
127+
const srcTotal = srcSession.getLength() * lineHeight;
128+
const srcVisible = source.renderer.$size.scrollerHeight;
129+
const srcMax = Math.max(0, srcTotal - srcVisible);
130+
const ratio = srcMax > 0 ? srcTop / srcMax : 0;
131+
132+
const tgtTotal = tgtSession.getLength() * lineHeight;
133+
const tgtVisible = target.renderer.$size.scrollerHeight;
134+
const tgtMax = Math.max(0, tgtTotal - tgtVisible);
135+
136+
tgtSession.setScrollTop(Math.round(ratio * tgtMax));
137+
scrollSyncing = false;
138+
};
139+
140+
editors.left.getSession().on('changeScrollTop', () => sync(editors.left, editors.right));
141+
editors.right.getSession().on('changeScrollTop', () => sync(editors.right, editors.left));
142+
}
143+
109144
function destroyDiff() {
110145
if (diffInstance) {
111146
diffInstance.destroy();
112147
diffInstance = null;
113148
}
149+
scrollSyncing = false;
114150
pendingLeft = null;
115151
pendingRight = null;
116152
if (container) {

ui/modules/things/thingsDiff.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export function ThingsDiff(targetTab) {
3434
let lastLeftJson: any = null;
3535
let lastRightJson: any = null;
3636
let syncing = false;
37+
let scrollSyncing = false;
3738
let fetchToken = 0;
3839
let activeProbe: { cancel: () => void } | null = null;
3940
let overviewSvg: SVGSVGElement | null = null;
@@ -176,6 +177,7 @@ export function ThingsDiff(targetTab) {
176177
diffInstance.destroy();
177178
diffInstance = null;
178179
}
180+
scrollSyncing = false;
179181
overviewSvg = null;
180182
if (dom.diffContainer) {
181183
dom.diffContainer.textContent = '';
@@ -373,8 +375,12 @@ export function ThingsDiff(targetTab) {
373375
function createOrUpdateDiff(leftContent: string, rightContent: string) {
374376
if (diffInstance) {
375377
const editors = diffInstance.getEditors();
378+
scrollSyncing = true;
376379
editors.left.setValue(leftContent, -1);
377380
editors.right.setValue(rightContent, -1);
381+
editors.left.getSession().setScrollTop(0);
382+
editors.right.getSession().setScrollTop(0);
383+
scrollSyncing = false;
378384
diffInstance.diff();
379385
renderChangeOverview(diffInstance.diffs);
380386
} else {
@@ -384,7 +390,7 @@ export function ThingsDiff(targetTab) {
384390
mode: 'ace/mode/json',
385391
theme: null,
386392
diffGranularity: 'specific',
387-
lockScrolling: true,
393+
lockScrolling: false,
388394
showDiffs: true,
389395
showConnectors: true,
390396
charDiffs: true,
@@ -401,9 +407,48 @@ export function ThingsDiff(targetTab) {
401407
},
402408
onDiffReady: renderChangeOverview,
403409
});
410+
setupScrollSync();
404411
}
405412
}
406413

414+
/**
415+
* Sets up proportional scroll synchronization between the two diff editors.
416+
* AceDiff's built-in lockScrolling uses a throttled (16ms) handler with a
417+
* synchronous re-entrancy guard, which allows a reverse-sync feedback loop
418+
* because the guard resets before the throttled reverse handler fires.
419+
* Our implementation uses direct (un-throttled) handlers, so the guard is
420+
* still active when setScrollTop fires the reverse changeScrollTop event
421+
* synchronously, reliably blocking the feedback loop.
422+
*/
423+
function setupScrollSync() {
424+
if (!diffInstance) return;
425+
const editors = diffInstance.getEditors();
426+
427+
const sync = (source, target) => {
428+
if (scrollSyncing) return;
429+
scrollSyncing = true;
430+
431+
const srcSession = source.getSession();
432+
const tgtSession = target.getSession();
433+
const srcTop = srcSession.getScrollTop();
434+
const lineHeight = source.renderer.lineHeight || 16;
435+
const srcTotal = srcSession.getLength() * lineHeight;
436+
const srcVisible = source.renderer.$size.scrollerHeight;
437+
const srcMax = Math.max(0, srcTotal - srcVisible);
438+
const ratio = srcMax > 0 ? srcTop / srcMax : 0;
439+
440+
const tgtTotal = tgtSession.getLength() * lineHeight;
441+
const tgtVisible = target.renderer.$size.scrollerHeight;
442+
const tgtMax = Math.max(0, tgtTotal - tgtVisible);
443+
444+
tgtSession.setScrollTop(Math.round(ratio * tgtMax));
445+
scrollSyncing = false;
446+
};
447+
448+
editors.left.getSession().on('changeScrollTop', () => sync(editors.left, editors.right));
449+
editors.right.getSession().on('changeScrollTop', () => sync(editors.right, editors.left));
450+
}
451+
407452
/**
408453
* Renders a change overview minimap in the ace-diff gutter.
409454
* Each diff region is shown as a colored rectangle positioned proportionally

ui/modules/things/thingsHistory.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@ function onThingChanged(thing, isNewThingId: boolean) {
111111
}
112112

113113
if (isNewThingId && Things.isHistoryModeActive()) {
114+
if (debounceTimer) {
115+
clearTimeout(debounceTimer);
116+
debounceTimer = null;
117+
}
118+
cancelActiveProbe();
114119
Things.setHistoryMode(false);
115120
dom.historyModeSwitch.checked = false;
116121
hideHistoryControls();
@@ -305,7 +310,7 @@ function debouncedFetch() {
305310
}
306311

307312
function fetchHistoricalState() {
308-
if (!currentThingId) return;
313+
if (!currentThingId || !Things.isHistoryModeActive()) return;
309314

310315
if (dom.historyRadioRevision.checked) {
311316
const revision = parseInt(dom.historyRevisionSlider.value, 10);

0 commit comments

Comments
 (0)