Skip to content

Commit feae2a6

Browse files
JohnMcLearclaude
andcommitted
fix(editor): auto-hide menu_right on readonly pads, accept showMenuRight=true override
Addresses Qodo review feedback on #7553: 1. Readonly pads now hide the right-side toolbar automatically. The original issue (#5182) was specifically about readonly embeds; the previous implementation only honoured an explicit `?showMenuRight=false` URL parameter, which meant that vanilla readonly pads still showed import/export/timeslider/settings/share/users controls — all noise for viewers who can't interact with the pad anyway. 2. Callers who still want the menu visible on readonly pads can opt back in with `?showMenuRight=true`. The URL-param callback now accepts both values instead of just `false`. 3. The Playwright spec's `browser.newContext() + clearCookies()` pattern was a no-op because the test navigated with the existing `page` fixture (different context). Switch to `page.context().clearCookies()`, and cover both the auto-hide and the explicit-override paths on a readonly-URL navigation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d48a4f3 commit feae2a6

2 files changed

Lines changed: 43 additions & 6 deletions

File tree

src/static/js/pad.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,19 @@ const getParameters = [
7777
},
7878
},
7979
{
80+
// showMenuRight accepts 'true' or 'false'. Explicit 'false' hides the
81+
// right-side toolbar (import/export/timeslider/settings/share/users);
82+
// explicit 'true' forces it visible, overriding the readonly
83+
// auto-hide applied further down (issue #5182). Any other value is
84+
// a no-op — the menu stays in its default state.
8085
name: 'showMenuRight',
81-
checkVal: 'false',
86+
checkVal: null,
8287
callback: (val) => {
83-
$('#editbar .menu_right').hide();
88+
if (val === 'false') {
89+
$('#editbar .menu_right').hide();
90+
} else if (val === 'true') {
91+
$('#editbar .menu_right').show();
92+
}
8493
},
8594
},
8695
{
@@ -685,6 +694,14 @@ const pad = {
685694
$('#chaticon').hide();
686695
$('#options-chatandusers').parent().hide();
687696
$('#options-stickychat').parent().hide();
697+
// Hide the right-side toolbar on readonly pads — import/export,
698+
// timeslider, settings, share, users are all noise for viewers
699+
// who can't interact with the pad. Callers who need those
700+
// controls visible on a readonly pad can force them back via
701+
// `?showMenuRight=true`, which runs in getParameters() above.
702+
if (getUrlVars().get('showMenuRight') !== 'true') {
703+
$('#editbar .menu_right').hide();
704+
}
688705
} else if (!settings.hideChat) { $('#chaticon').show(); }
689706

690707
$('body').addClass(window.clientVars.readonly ? 'readonly' : 'readwrite');

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

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import {expect, test} from "@playwright/test";
22
import {appendQueryParams, goToNewPad} from "../helper/padHelper";
33

4-
test.beforeEach(async ({page, browser}) => {
5-
const context = await browser.newContext();
6-
await context.clearCookies();
4+
test.beforeEach(async ({page}) => {
5+
// clearCookies on the page's own context — creating a separate
6+
// BrowserContext and clearing cookies on it is a no-op for the page
7+
// fixture (Qodo review feedback on #7553).
8+
await page.context().clearCookies();
79
await goToNewPad(page);
810
});
911

@@ -19,8 +21,26 @@ test.describe('showMenuRight URL parameter', function () {
1921
await expect(page.locator('#editbar .menu_left')).toBeVisible();
2022
});
2123

22-
test('showMenuRight with any other value leaves .menu_right visible', async function ({page}) {
24+
test('showMenuRight=true keeps .menu_right visible', async function ({page}) {
2325
await appendQueryParams(page, {showMenuRight: 'true'});
2426
await expect(page.locator('#editbar .menu_right')).toBeVisible();
2527
});
28+
29+
test('readonly pad hides .menu_right by default', async function ({page}) {
30+
// Find the share link which exposes the readonly r.* id, then navigate.
31+
await page.locator('.buttonicon-embed').click();
32+
const readonlyUrl = await page.locator('#readonlyInput').inputValue();
33+
expect(readonlyUrl).toMatch(/\/p\/r\./);
34+
await page.goto(readonlyUrl);
35+
await page.waitForSelector('#editorcontainer.initialized');
36+
await expect(page.locator('#editbar .menu_right')).toBeHidden();
37+
});
38+
39+
test('readonly pad with showMenuRight=true keeps the menu visible', async function ({page}) {
40+
await page.locator('.buttonicon-embed').click();
41+
const readonlyUrl = await page.locator('#readonlyInput').inputValue();
42+
await page.goto(`${readonlyUrl}?showMenuRight=true`);
43+
await page.waitForSelector('#editorcontainer.initialized');
44+
await expect(page.locator('#editbar .menu_right')).toBeVisible();
45+
});
2646
});

0 commit comments

Comments
 (0)