Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions src/static/css/timeslider.css
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,33 @@
overflow-y: auto;
}
#outerdocbody {
display: block;
display: flex;
flex-direction: row;
justify-content: center;
align-items: flex-start;
width: 100%;
}

#outerdocbody > #sidediv {
flex: 0 0 auto;
padding-top: 30px;
padding-bottom: 30px;
}

#outerdocbody > #innerdocbody {
flex: 1 1 auto;
min-width: 0;
padding-right: calc(var(--editor-horizontal-padding, 0px) + 15px);
padding-top: 30px;
padding-left: calc(var(--editor-horizontal-padding, 0px) + 15px);
padding-bottom: 30px;
}

#innerdocbody {
white-space: normal;
word-break: break-word;
width: 100%;
margin: 0 auto;
margin: 0;
height: auto;
}

Expand All @@ -151,4 +169,4 @@
display: flex;
flex-wrap: wrap;
}
}
}
73 changes: 73 additions & 0 deletions src/static/js/broadcast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,72 @@ const loadBroadcastJS = (socket, sendSocketMsg, fireWhenAllScriptsAreLoaded, Bro
},
};

const targetBody = document.getElementById('innerdocbody');
const sideDiv = document.getElementById('sidediv');
const sideDivInner = document.getElementById('sidedivinner');
const appendNewSideDivLine = () => {
const lineDiv = document.createElement('div');
sideDivInner.appendChild(lineDiv);
const lineSpan = document.createElement('span');
lineSpan.classList.add('line-number');
lineSpan.appendChild(document.createTextNode(sideDivInner.children.length));
lineDiv.appendChild(lineSpan);
};

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);
}
}

const newNumLines = Math.max(targetBody.children.length, 1);
while (sideDivInner.children.length < newNumLines) appendNewSideDivLine();
while (sideDivInner.children.length > newNumLines) sideDivInner.lastElementChild.remove();
for (const [i, sideDivLine] of Array.prototype.entries.call(sideDivInner.children)) {
sideDivLine.style.height = `${lineOffsets[i]}px`;
sideDivLine.style.lineHeight = `${lineHeights[i]}px`;
}
$(sideDiv).addClass('sidedivdelayed');
};

let lineNumberUpdatePending = false;
const scheduleLineNumberUpdate = () => {
if (lineNumberUpdatePending) return;
lineNumberUpdatePending = true;
window.requestAnimationFrame(() => {
window.requestAnimationFrame(() => {
lineNumberUpdatePending = false;
updateLineNumbers();
});
});
};

const applyChangeset = (changeset, revision, preventSliderMovement, timeDelta) => {
// disable the next 'gotorevision' call handled by a timeslider update
if (!preventSliderMovement) {
Expand Down Expand Up @@ -194,6 +260,7 @@ const loadBroadcastJS = (socket, sendSocketMsg, fireWhenAllScriptsAreLoaded, Bro

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

Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
updateTimer();

Expand Down Expand Up @@ -465,6 +532,12 @@ const loadBroadcastJS = (socket, sendSocketMsg, fireWhenAllScriptsAreLoaded, Bro
padContents.currentDivs.push(div);
$('#innerdocbody').append(div);
}
updateLineNumbers();
scheduleLineNumberUpdate();
$(window).on('resize', scheduleLineNumberUpdate);
window.addEventListener('load', scheduleLineNumberUpdate, {once: true});
document.fonts?.ready?.then(scheduleLineNumberUpdate);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Fonts could change the line numbers too.

$('#viewfontmenu').on('change', () => window.setTimeout(scheduleLineNumberUpdate, 0));
});

// this is necessary to keep infinite loops of events firing,
Expand Down
39 changes: 38 additions & 1 deletion src/static/js/timeslider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,37 @@ let token, padId, exportLinks, socket, changesetLoader, BroadcastSlider;
let cp = '';
const playbackSpeedCookie = 'timesliderPlaybackSpeed';

const getPrefsCookieName = () => `${cp}${window.location.protocol === 'https:' ? 'prefs' : 'prefsHttp'}`;

const readPadPrefs = () => {
try {
let json = Cookies.get(getPrefsCookieName());
if (json == null) {
const unprefixed = window.location.protocol === 'https:' ? 'prefs' : 'prefsHttp';
if (unprefixed !== getPrefsCookieName()) json = Cookies.get(unprefixed);
}
return json == null ? {} : JSON.parse(json);
} catch (err) {
return {};
}
};

const writePadPrefs = (prefs) => {
Cookies.set(getPrefsCookieName(), JSON.stringify(prefs), {expires: 365 * 100});
};

const setPadPref = (prefName, value) => {
const prefs = readPadPrefs();
prefs[prefName] = value;
writePadPrefs(prefs);
};

const applyShowLineNumbers = (showLineNumbers) => {
padutils.setCheckbox($('#options-linenoscheck'), showLineNumbers);
$('body').toggleClass('line-numbers-hidden', !showLineNumbers);
window.requestAnimationFrame(() => $(window).trigger('resize'));
};

const init = () => {
padutils.setupGlobalExceptionHandler();
$(document).ready(() => {
Expand Down Expand Up @@ -113,7 +144,7 @@ const fireWhenAllScriptsAreLoaded = [];
const handleClientVars = (message) => {
// save the client Vars
window.clientVars = message.data;
cp = window.clientVars.cookiePrefix || '';
cp = (window as any).clientVars?.cookiePrefix || '';

if (window.clientVars.sessionRefreshInterval) {
const ping =
Expand Down Expand Up @@ -169,6 +200,12 @@ const handleClientVars = (message) => {
$('#playpause_button_icon').attr('title', html10n.get('timeslider.playPause'));
$('#leftstep').attr('title', html10n.get('timeslider.backRevision'));
$('#rightstep').attr('title', html10n.get('timeslider.forwardRevision'));
padutils.bindCheckboxChange($('#options-linenoscheck'), () => {
const showLineNumbers = padutils.getCheckbox('#options-linenoscheck');
setPadPref('showLineNumbers', showLineNumbers);
applyShowLineNumbers(showLineNumbers);
});
applyShowLineNumbers(readPadPrefs().showLineNumbers !== false);
Comment on lines +203 to +208
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

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


// font family change
$('#viewfontmenu').on('change', function () {
Expand Down
12 changes: 11 additions & 1 deletion src/static/skins/colibris/timeslider.css
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,16 @@
font-size: .9em;
}

.timeslider #outerdocbody > #sidediv {
padding-top: 30px;
padding-bottom: 30px;
}

.timeslider #outerdocbody > #innerdocbody {
padding-top: 30px;
padding-bottom: 30px;
}

@media (max-width: 800px) {

#slider-btn-container {
Expand All @@ -95,4 +105,4 @@
#slider-btn-container #playpause_button_icon:before {
font-size: 18px;
}
}
}
8 changes: 8 additions & 0 deletions src/templates/timeslider.html
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,12 @@ <h1 class="timeslider-title">
<!----------------------------->

<div id="outerdocbody">
<div id="sidediv" class="sidediv">
<div id="sidedivinner" class="sidedivinner"></div>
</div>
<div id="innerdocbody">
</div>
<div id="linemetricsdiv">x</div>
</div>


Expand Down Expand Up @@ -248,6 +252,10 @@ <h1 data-l10n-id="pad.settings.padSettings"></h1>
<option value="1000" data-l10n-id="timeslider.settings.playbackSpeed.1000ms">1000 ms</option>
</select>
</p>
<p>
<input type="checkbox" id="options-linenoscheck">
<label for="options-linenoscheck" data-l10n-id="pad.settings.linenocheck"></label>
</p>
</div></div>
</div>
</body>
Expand Down
72 changes: 72 additions & 0 deletions src/tests/frontend-new/specs/timeslider_line_numbers.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import {expect, test} from "@playwright/test";
import {clearPadContent, goToNewPad, writeToPad} from "../helper/padHelper";
import {showSettings} from "../helper/settingsHelper";

test.describe('timeslider line numbers', function () {
test.beforeEach(async ({context}) => {
await context.clearCookies();
});

test('shows line numbers aligned with the rendered document lines', async function ({page}) {
const padId = await goToNewPad(page);
await clearPadContent(page);
await writeToPad(page, 'One\nTwo\nThree');
await page.waitForTimeout(1000);

await page.goto(`http://localhost:9001/p/${padId}/timeslider`);
await page.waitForSelector('#timeslider-wrapper', {state: 'visible'});
await page.waitForSelector('#sidediv.sidedivdelayed', {state: 'attached'});
await page.waitForTimeout(1000);

await expect(page.locator('#sidediv')).toBeVisible();
await expect(page.locator('#sidediv .line-number').nth(0)).toHaveText('1');
await expect(page.locator('#sidediv .line-number').nth(1)).toHaveText('2');
await expect(page.locator('#sidediv .line-number').nth(2)).toHaveText('3');

const counts = await page.evaluate(() => ({
docLines: document.querySelector('#innerdocbody')?.children.length,
gutterLines: document.querySelector('#sidedivinner')?.children.length,
}));
expect(counts.gutterLines).toBe(counts.docLines);

const alignment = await page.evaluate(() => {
const innerdocbody = document.querySelector('#innerdocbody');
const sidediv = document.querySelector('#sidediv');
const docLines = [...document.querySelectorAll('#innerdocbody > div')];
const gutterLines = [...document.querySelectorAll('#sidedivinner > div')];
const sideRect = sidediv?.getBoundingClientRect();
const innerRect = innerdocbody?.getBoundingClientRect();
return {
gap: sideRect && innerRect ? Math.abs(innerRect.left - sideRect.right) : null,
};
});

expect(alignment.gap).not.toBeNull();
expect(alignment.gap!).toBeLessThanOrEqual(2);
});

test('inherits and persists the line-number preference from the shared cookie', async function ({page}) {
const padId = await goToNewPad(page);
await page.context().addCookies([{
name: 'prefsHttp',
value: encodeURIComponent(JSON.stringify({showLineNumbers: false})),
url: 'http://localhost:9001',
}]);

await page.goto(`http://localhost:9001/p/${padId}/timeslider`);
await page.waitForSelector('#timeslider-wrapper', {state: 'visible'});
await showSettings(page);

await expect(page.locator('#options-linenoscheck')).not.toBeChecked();
await expect(page.locator('body')).toHaveClass(/line-numbers-hidden/);

await page.locator('label[for="options-linenoscheck"]').click();
await expect(page.locator('#options-linenoscheck')).toBeChecked();
await expect(page.locator('body')).not.toHaveClass(/line-numbers-hidden/);

await page.reload();
await page.waitForSelector('#timeslider-wrapper', {state: 'visible'});
await expect(page.locator('#options-linenoscheck')).toBeChecked();
await expect(page.locator('body')).not.toHaveClass(/line-numbers-hidden/);
});
});
Loading