Skip to content

Commit b22bad0

Browse files
JohnMcLearclaude
andcommitted
fix(pad): keep menu_right visible on readonly pads by default
PR #X (issue #5182) added a client-side `$('#editbar .menu_right').hide()` for readonly pads, opt-out via `?showMenuRight=true`. The intent — clean chrome for iframe-embedded readonly announcement pads — was good but the implementation hid the userlist toggle along with import/export. That has two unwanted effects on non-embed deployments: * Plugins like ep_guest inject their "Log In" button into `#myuser` inside the userlist popup, which lives under `.menu_right`. When the guest user (readOnly: true) lands on a readonly pad, the button they need to escape readonly is hidden. Chicken-and-egg. * Settings, embed, home, timeslider, showusers are all legitimately useful for readonly viewers and got removed alongside the actual write-only controls. The server already does the right thing without help from the client: * src/node/utils/toolbar.ts:282-290 strips `savedrevision` from the right toolbar when isReadOnly is true. * src/static/css/pad/popup_import_export.css:1 has `.readonly .acl-write { display: none }`, which hides the Import column of the import/export popup. Export stays visible (and is legitimately useful in readonly). Drop the client-side blanket hide. The iframe-embed use case from #5182 is still served by `?showMenuRight=false` (the existing handler at src/static/js/pad.ts:91-107), and `?showControls=false` continues to hide the entire editbar for callers who want even the left menu gone. Tests: rewrite `hide_menu_right.spec.ts`. The "readonly pad hides .menu_right by default" assertion is inverted; a new test confirms `?showMenuRight=false` still hides on readonly pads. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 294158e commit b22bad0

2 files changed

Lines changed: 34 additions & 13 deletions

File tree

src/static/js/pad.ts

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,12 @@ const getParameters = [
9191
},
9292
{
9393
// showMenuRight accepts 'true' or 'false'. Explicit 'false' hides the
94-
// right-side toolbar (import/export/timeslider/settings/share/users);
95-
// explicit 'true' forces it visible, overriding the readonly
96-
// auto-hide applied further down (issue #5182). Any other value is
97-
// a no-op — the menu stays in its default state.
94+
// right-side toolbar (import/export/timeslider/settings/embed/home/
95+
// users) — useful for iframe-embedded readonly pads where the menu
96+
// is just chrome (issue #5182). Explicit 'true' forces the menu
97+
// visible (a no-op against the default, kept for symmetry and for
98+
// overriding any future caller-applied hide). Any other value is a
99+
// no-op — the menu stays in its default state.
98100
name: 'showMenuRight',
99101
checkVal: null,
100102
callback: (val) => {
@@ -787,14 +789,18 @@ const pad = {
787789
$('#chaticon').hide();
788790
$('#options-chatandusers').parent().hide();
789791
$('#options-stickychat').parent().hide();
790-
// Hide the right-side toolbar on readonly pads — import/export,
791-
// timeslider, settings, share, users are all noise for viewers
792-
// who can't interact with the pad. Callers who need those
793-
// controls visible on a readonly pad can force them back via
794-
// `?showMenuRight=true`, which runs in getParameters() above.
795-
if (getUrlVars().get('showMenuRight') !== 'true') {
796-
$('#editbar .menu_right').hide();
797-
}
792+
// The right-side toolbar stays visible on readonly pads. The
793+
// server-side `toolbar.menu(buttons, isReadOnly)` (see
794+
// src/node/utils/toolbar.ts) already strips `savedrevision`, and
795+
// `.readonly .acl-write { display: none }` hides the Import column
796+
// inside the import/export popup, so the remaining controls
797+
// (export, timeslider, settings, embed, home, showusers) are all
798+
// safe for readonly viewers — and the userlist is the surface that
799+
// plugins like ep_guest hang their "Log In" button off, so hiding
800+
// it traps guests in readonly with no way out. Iframe-embed use
801+
// cases that want a clean look (issue #5182) opt in to the hide
802+
// via `?showMenuRight=false`, or hide the whole editbar via
803+
// `?showControls=false`.
798804
} else if (!settings.hideChat) { $('#chaticon').show(); }
799805

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

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,25 @@ test.describe('showMenuRight URL parameter', function () {
4444
return url;
4545
};
4646

47-
test('readonly pad hides .menu_right by default', async function ({page}) {
47+
test('readonly pad shows .menu_right by default', async function ({page}) {
48+
// The server-side toolbar.menu(..., isReadOnly) strips `savedrevision`
49+
// and `.readonly .acl-write { display: none }` hides the Import
50+
// column of the import/export popup, so the remaining controls
51+
// (export, timeslider, settings, embed, home, showusers) are safe
52+
// for readonly viewers — and ep_guest / other auth plugins depend
53+
// on the userlist being reachable to surface their Log In button.
4854
const readonlyUrl = await getReadonlyUrl(page);
4955
await page.goto(readonlyUrl);
5056
await page.waitForSelector('#editorcontainer.initialized');
57+
await expect(page.locator('#editbar .menu_right')).toBeVisible();
58+
});
59+
60+
test('readonly pad with showMenuRight=false hides the menu', async function ({page}) {
61+
// Iframe-embed use case (issue #5182): the embedder opts in to the
62+
// hide via the URL parameter when they want a clean look.
63+
const readonlyUrl = await getReadonlyUrl(page);
64+
await page.goto(`${readonlyUrl}?showMenuRight=false`);
65+
await page.waitForSelector('#editorcontainer.initialized');
5166
await expect(page.locator('#editbar .menu_right')).toBeHidden();
5267
});
5368

0 commit comments

Comments
 (0)