feat: add timeslider line numbers

[JohnMcLear]

Summary

• add editor-style line numbers to the timeslider gutter
• add a timeslider setting that defaults from and persists to the shared line number cookie
• align the gutter/document spacing and cover the behavior with frontend tests

Closes #4669

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Add timeslider line numbers with persistence and alignment

✨ Enhancement

Walkthroughs

Description

• Add editor-style line numbers to timeslider gutter with dynamic alignment
• Implement line number preference persistence via shared cookie
• Update CSS layout to support side-by-side gutter and document display
• Add comprehensive frontend tests for line number functionality

Diagram

flowchart LR
  A["User opens timeslider"] --> B["Read line number preference from cookie"]
  B --> C["Apply show/hide line numbers setting"]
  C --> D["Render document with gutter"]
  D --> E["Calculate line heights and offsets"]
  E --> F["Display aligned line numbers"]
  F --> G["User toggles line numbers in settings"]
  G --> H["Save preference to cookie"]
  H --> I["Update display and trigger resize"]

Loading

File Changes

1. src/static/js/broadcast.ts ✨ Enhancement +67/-0

Implement line number calculation and synchronization logic

• Add updateLineNumbers() function to calculate line heights and offsets for gutter alignment
• Add appendNewSideDivLine() helper to create new line number elements
• Add scheduleLineNumberUpdate() to debounce updates using requestAnimationFrame
• Integrate line number updates on changeset application and document initialization
• Attach resize, load, and font change event listeners to trigger updates

src/static/js/broadcast.ts

---

2. src/static/js/timeslider.ts ✨ Enhancement +38/-0

Add preference persistence and UI binding for line numbers

• Add cookie preference management functions: readPadPrefs(), writePadPrefs(), setPadPref()
• Add applyShowLineNumbers() to toggle line number visibility and trigger layout updates
• Implement checkbox binding to persist line number preference changes
• Initialize line number visibility from stored preference on page load
• Extract and store cookie prefix from client variables

src/static/js/timeslider.ts

---

3. src/tests/frontend-new/specs/timeslider_line_numbers.spec.ts 🧪 Tests +72/-0

Add comprehensive frontend tests for line numbers

• Add test verifying line numbers display and alignment with document lines
• Add test confirming gutter line count matches document line count
• Add test validating visual alignment gap between gutter and document
• Add test for preference inheritance from shared cookie and persistence across reloads

src/tests/frontend-new/specs/timeslider_line_numbers.spec.ts

---

View more (3)

4. src/static/css/timeslider.css ⚙️ Configuration changes +21/-3

Update layout to flexbox for gutter and document alignment

• Change #outerdocbody from block to flex layout with row direction
• Add flex properties to #sidediv for fixed-width gutter positioning
• Add flex properties and padding to #innerdocbody for responsive content area
• Update padding and margin values to support centered layout with proper spacing

src/static/css/timeslider.css

---

5. src/static/skins/colibris/timeslider.css ⚙️ Configuration changes +11/-1

Add colibris skin styling for line number gutter

• Add padding rules for #sidediv and #innerdocbody in colibris skin
• Ensure consistent spacing in themed timeslider layout

src/static/skins/colibris/timeslider.css

---

6. src/templates/timeslider.html ✨ Enhancement +8/-0

Add HTML structure for line number gutter and settings

• Add #sidediv container with #sidedivinner for line number elements
• Add #linemetricsdiv helper element for font metrics calculation
• Add checkbox input #options-linenoscheck to settings panel
• Add label for line numbers checkbox with localization support

src/templates/timeslider.html

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

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

1. Timeslider line numbers not gated 📘 Rule violation ≡ Correctness

Description

The new timeslider line-number feature is enabled by default and is not behind a feature flag.
Additionally, line-number DOM updates still run even when the user preference hides them, so the
disabled path does not match pre-change behavior.

Code

src/static/js/timeslider.ts[R202-207]

+  padutils.bindCheckboxChange($('#options-linenoscheck'), () => {
+    const showLineNumbers = padutils.getCheckbox('#options-linenoscheck');
+    setPadPref('showLineNumbers', showLineNumbers);
+    applyShowLineNumbers(showLineNumbers);
+  });
+  applyShowLineNumbers(readPadPrefs().showLineNumbers !== false);

Evidence

PR Compliance ID 8 requires new features to be behind a feature flag, disabled by default, and to
preserve the pre-existing code path when disabled. The code defaults to showing line numbers
(readPadPrefs().showLineNumbers !== false) and the broadcast logic calls updateLineNumbers()
unconditionally during playback and initialization, even if line numbers are hidden via a CSS class
toggle.

src/static/js/timeslider.ts[202-207]
src/static/js/broadcast.ts[254-257]
src/static/js/broadcast.ts[526-534]
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
Timeslider line numbers are a new feature but are enabled by default and not gated behind a feature flag. The line-number update logic also runs even when the preference hides line numbers, so the disabled path does not preserve the pre-existing behavior.
## Issue Context
Compliance requires new features to be behind a feature flag and disabled by default, and when disabled the executed code path should match pre-change behavior.
## Fix Focus Areas
- src/static/js/timeslider.ts[202-207]
- src/static/js/broadcast.ts[254-257]
- src/static/js/broadcast.ts[526-534]
- src/templates/timeslider.html[113-118]
- src/static/css/timeslider.css[123-143]

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

---

2. Unthrottled line-number recalcs ☑ 🐞 Bug ➹ Performance

Description

broadcast.ts calls updateLineNumbers() synchronously on every applyChangeset(), forcing repeated
full-document layout reads and DOM writes during timeslider scrubbing/playback and potentially
making large pads very slow. scheduleLineNumberUpdate() also doesn’t coalesce repeated calls, so
resize can queue many redundant recalculations.

Code

src/static/js/broadcast.ts[R254-258]

 padContents.currentRevision = revision;
 padContents.currentTime += timeDelta;
+    updateLineNumbers();

Evidence

The timeslider’s updateLineNumbers() does a full walk of #innerdocbody children, reads layout
(offsetTop/clientHeight/getComputedStyle) and then writes styles for every gutter row; this is
called directly from applyChangeset() after each revision update. In contrast, the editor runs its
analogous updateLineNumbers() from an idle work timer with a time budget, indicating this work is
expected to be throttled/coalesced rather than run per operation.

src/static/js/broadcast.ts[135-187]
src/static/js/broadcast.ts[189-193]
src/static/js/broadcast.ts[195-259]
src/static/js/broadcast.ts[520-535]
src/static/js/ace2_inner.ts[782-818]

Agent prompt

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

## Issue description
`updateLineNumbers()` is invoked in a hot path (per changeset) and resize scheduling is not coalesced. This causes excessive layout/reflow work, especially when scrubbing through many revisions or resizing the window.
## Issue Context
The editor’s gutter update logic is run via an idle timer with a time budget; the timeslider should similarly throttle/coalesce updates.
## Fix Focus Areas
- src/static/js/broadcast.ts[189-193]
- src/static/js/broadcast.ts[195-259]
- src/static/js/broadcast.ts[520-535]
## Proposed fix approach
- Add a `let lineNumberUpdatePending = false` flag.
- Change `scheduleLineNumberUpdate()` to no-op if `lineNumberUpdatePending` is true; otherwise set it true and run a single `requestAnimationFrame(() => { lineNumberUpdatePending = false; updateLineNumbers(); })` (keep double-rAF only if you can justify it, but still coalesce).
- Replace direct `updateLineNumbers()` calls in `applyChangeset()` with `scheduleLineNumberUpdate()` (or call direct only when needed and still coalesce).
- Ensure initial render still triggers at least one update after DOM insertion.

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

---

3. updateLineNumbers() lacks comments 📘 Rule violation ⚙ Maintainability

Description

Complex, non-obvious layout logic was added for computing gutter line heights/offsets without
explanatory comments. This makes future maintenance and bug-fixing riskier.

Code

src/static/js/broadcast.ts[R147-177]

+  const updateLineNumbers = () => {
+    if (!targetBody || !sideDiv || !sideDivInner) return;
+    const lineOffsets = [];
+    const lineHeights = [];
+    const innerdocbodyStyles = getComputedStyle(targetBody);
+    const defaultLineHeight = parseInt(innerdocbodyStyles.lineHeight);
+
+    for (const docLine of targetBody.children) {
+      let height;
+      const nextDocLine = docLine.nextElementSibling;
+      if (nextDocLine) {
+        if (lineOffsets.length === 0) {
+          height = nextDocLine.offsetTop - parseInt(
+              innerdocbodyStyles.getPropertyValue('padding-top'));
+        } else {
+          height = nextDocLine.offsetTop - docLine.offsetTop;
+        }
+      } else {
+        height = docLine.clientHeight || docLine.offsetHeight;
+      }
+      lineOffsets.push(height);
+
+      if (docLine.clientHeight !== defaultLineHeight && docLine.firstElementChild != null) {
+        const elementStyle = window.getComputedStyle(docLine.firstElementChild);
+        const lineHeight = parseInt(elementStyle.getPropertyValue('line-height'));
+        const marginBottom = parseInt(elementStyle.getPropertyValue('margin-bottom'));
+        lineHeights.push(lineHeight + marginBottom);
+      } else {
+        lineHeights.push(defaultLineHeight);
+      }
+    }

Evidence

PR Compliance ID 10 requires comments for complex or non-obvious code. The added logic in
updateLineNumbers() performs multi-branch DOM measurements and special casing (including
padding/line-height/margins) without any comment describing the intent or invariants.

src/static/js/broadcast.ts[147-177]
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 newly added `updateLineNumbers()` logic contains non-obvious DOM-measurement/layout computations but no comments explaining the approach.
## Issue Context
Future contributors will likely need to adjust this logic for fonts, padding, and rendering differences; comments should explain why the measurements are done this way and what assumptions are being made.
## Fix Focus Areas
- src/static/js/broadcast.ts[147-177]
- src/static/js/broadcast.ts[189-193]

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

---

4. Prefs cookie not normalized 🐞 Bug ⚙ Maintainability

Description

timeslider.ts rewrites the entire prefs cookie object when toggling line numbers, retaining any
existing keys instead of applying the same normalization the main pad performs. This can keep legacy
keys (that pad_cookie explicitly strips) in the long-lived prefs cookie when users change the
timeslider setting.

Code

src/static/js/timeslider.ts[R53-61]

+const writePadPrefs = (prefs) => {
+  Cookies.set(getPrefsCookieName(), JSON.stringify(prefs), {expires: 365 * 100});
+};
+
+const setPadPref = (prefName, value) => {
+  const prefs = readPadPrefs();
+  prefs[prefName] = value;
+  writePadPrefs(prefs);
+};

Evidence

The new timeslider code reads the whole prefs object, mutates it, and writes it back unchanged
except for the updated field. The main pad’s cookie initialization explicitly deletes certain keys
before writing, so the timeslider path diverges from the established cookie-normalization behavior
when it persists prefs.

src/static/js/timeslider.ts[40-61]
src/static/js/pad_cookie.ts[28-33]
src/static/js/pad_cookie.ts[60-62]

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 timeslider preference write path persists the entire prefs object as-is, which diverges from the pad’s cookie normalization behavior and can keep unwanted legacy keys indefinitely.
## Issue Context
`pad_cookie.init()` deletes some keys before persisting the prefs cookie. The timeslider currently does not do an equivalent normalization step when it writes prefs.
## Fix Focus Areas
- src/static/js/timeslider.ts[53-61]
## Proposed fix approach
- Before calling `writePadPrefs(prefs)`, delete the same normalized keys the pad deletes (at minimum: `userId`, `name`, `colorId`) or refactor to reuse the shared cookie helper (pad_cookie) rather than duplicating logic.
- Keep behavior the same for `showLineNumbers`, only normalize extra keys.

ⓘ 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]

1. Timeslider line numbers not gated 📘 Rule violation ≡ Correctness

The new timeslider line-number feature is enabled by default and is not behind a feature flag.
Additionally, line-number DOM updates still run even when the user preference hides them, so the
disabled path does not match pre-change behavior.

Agent Prompt

## Issue description
Timeslider line numbers are a new feature but are enabled by default and not gated behind a feature flag. The line-number update logic also runs even when the preference hides line numbers, so the disabled path does not preserve the pre-existing behavior.

## Issue Context
Compliance requires new features to be behind a feature flag and disabled by default, and when disabled the executed code path should match pre-change behavior.

## Fix Focus Areas
- src/static/js/timeslider.ts[202-207]
- src/static/js/broadcast.ts[254-257]
- src/static/js/broadcast.ts[526-534]
- src/templates/timeslider.html[113-118]
- src/static/css/timeslider.css[123-143]

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

[SamTV12345]

Good idea. Fonts could change the line numbers too.

[SamTV12345]

could you update the mr title? I think it also adds a configurable cookie for line numbering.

[JohnMcLear]

could you update the mr title? I think it also adds a configurable cookie for line numbering.

mr title?