Skip to content

Commit 193d59c

Browse files
thjaeckleclaude
andcommitted
Address PR review findings for history exploration feature
Fix all blockers, major, and minor issues from hu-ahmed's review: Blockers: - B1: Make tab-sync guard async-safe via setTimeout(0) to survive Bootstrap's async shown.bs.tab events - B2: Fix toDatetimeLocalValue to properly convert UTC to local timezone, preventing 404s for non-UTC users - B3: Add Environments.addChangeListener to thingsHistory and thingUpdates to close SSE streams on environment switch Majors: - M1: Add fetch token to prevent stale diff responses from overwriting newer state - M2: Wrap JSON.parse in onHistoricalMessage in try/catch - M3: Validate from <= to range before opening SSE stream - M4: Add missing .max constraint on historicalFromRevision input - M5: Add thingId guard in thingsHistory probe callback - M6: Restore Ace.Editor type annotations in policy files - M7: Scope number input CSS to history/diff containers - M9: Add MAX_MESSAGES cap (5000) with user notification - M10: Use closest('tr') for robust row click handling - M11: Return promises from refreshThingAtRevision/Timestamp, update banner on error - M12: Call thingsDiff.ready() before registering listeners - M13: Show toast on unexpected historical stream error Minors: - Extract shared probeOldestRevision helper to things.ts - Extract shared toDatetimeLocalValue to utils.ts - Deduplicate HISTORY_FIELDS constant (export from things.ts) - Use structuredClone instead of JSON roundtrip - Make historyModeActive a getter (isHistoryModeActive) - Fix Term[] return type (was mistyped [Term?] tuple) - Use Bootstrap Collapse API instead of classList manipulation - Add AceDiff destroy() support, reuse SVG in minimap - Pin ace-diff version exactly (4.0.0) - Add null safety for sibling.parentElement - Type messages/filteredMessages as any[] - Cast ace as proper type instead of any - Remove unused Time Travel button from Diff view - Fix minimap click scroll bounce (disable animation) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3c76339 commit 193d59c

15 files changed

Lines changed: 373 additions & 234 deletions

ui/main.scss

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,15 +323,18 @@ button.busy .spinner-border {
323323
}
324324

325325
/* Revision picker alignment: force fixed sizes to align stacked rows */
326-
.input-group input[type="number"] {
326+
#collapseThings .input-group input[type="number"],
327+
#collapseThingUpdates .input-group input[type="number"] {
327328
flex: 0 0 80px !important;
328329
width: 80px !important;
329330
padding-right: 6px !important;
330331
-moz-appearance: textfield !important;
331332
}
332333

333-
.input-group input[type="number"]::-webkit-outer-spin-button,
334-
.input-group input[type="number"]::-webkit-inner-spin-button {
334+
#collapseThings .input-group input[type="number"]::-webkit-outer-spin-button,
335+
#collapseThings .input-group input[type="number"]::-webkit-inner-spin-button,
336+
#collapseThingUpdates .input-group input[type="number"]::-webkit-outer-spin-button,
337+
#collapseThingUpdates .input-group input[type="number"]::-webkit-inner-spin-button {
335338
-webkit-appearance: none;
336339
margin: 0;
337340
}

ui/main.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,8 @@ document.addEventListener('DOMContentLoaded', async function() {
9595
itemsId: 'tabItemsThing',
9696
contentId: 'tabContentThing',
9797
});
98-
Things.addChangeListener(thingsDiff.onThingChanged);
99-
Things.addHistoryModeChangeListener(thingsDiff.onHistoryModeChanged);
10098
await thingsDiff.ready();
99+
Things.addChangeListener(thingsDiff.onThingChanged);
101100

102101
const attributesDiff = SubDiff({
103102
itemsId: 'tabItemsAttribute',

ui/modules/policies/policiesJSON.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313

1414
import * as ace from 'ace-builds/src-noconflict/ace';
15+
import { Ace } from 'ace-builds';
1516
import * as API from '../api.js';
1617
import * as Environments from '../environments/environments.js'
1718
import * as Utils from '../utils.js';
@@ -30,7 +31,7 @@ let dom: DomElements = {
3031
selectPolicyJSONTemplate: null,
3132
};
3233

33-
let policyEditor;
34+
let policyEditor: Ace.Editor;
3435

3536
export function ready() {
3637

ui/modules/policies/policiesResources.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313

1414
import * as ace from 'ace-builds/src-noconflict/ace';
15+
import { Ace } from 'ace-builds';
1516
import * as API from '../api.js';
1617
import * as Utils from '../utils.js';
1718
import { CrudOperation, CrudToolbar } from '../utils/crudToolbar.js';
@@ -35,7 +36,7 @@ let dom : DomElements = {
3536
crudResource: null,
3637
} ;
3738

38-
let resourceEditor;
39+
let resourceEditor: Ace.Editor;
3940

4041
export function ready() {
4142
Utils.getAllElementsById(dom);

ui/modules/policies/policiesSubjects.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313

1414
import * as ace from 'ace-builds/src-noconflict/ace';
15+
import { Ace } from 'ace-builds';
1516
import * as API from '../api.js';
1617
import * as Utils from '../utils.js';
1718
import { CrudOperation, CrudToolbar } from '../utils/crudToolbar.js';
@@ -33,7 +34,7 @@ let dom: DomElements = {
3334
crudSubject: null,
3435
} ;
3536

36-
let subjectEditor;
37+
let subjectEditor: Ace.Editor;
3738

3839
export function ready() {
3940
Utils.getAllElementsById(dom);

ui/modules/things/subDiff.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export function SubDiff(targetTab, title: string) {
2929
let pendingRight: string | null = null;
3030
let onActivatedCallback: ((link: HTMLAnchorElement) => void) | null = null;
3131

32-
return { ready, update, getTabLink, setOnActivated };
32+
return { ready, update, getTabLink, setOnActivated, destroy: destroyDiff };
3333

3434
async function ready() {
3535
const tabId = Utils.addTab(
@@ -82,7 +82,7 @@ export function SubDiff(targetTab, title: string) {
8282
diffInstance.diff();
8383
} else {
8484
diffInstance = new AceDiff({
85-
ace: ace as any,
85+
ace: ace as unknown as { edit: typeof ace.edit },
8686
element: container,
8787
mode: 'ace/mode/json',
8888
theme: null,
@@ -105,4 +105,11 @@ export function SubDiff(targetTab, title: string) {
105105
});
106106
}
107107
}
108+
109+
function destroyDiff() {
110+
if (diffInstance) {
111+
diffInstance.destroy();
112+
diffInstance = null;
113+
}
114+
}
108115
}

0 commit comments

Comments
 (0)