Skip to content

feat: Improve Admin Settings UX and restore inline comments (#7603) #7666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
AyushiGupta160604 wants to merge 2 commits into
base: develop
Choose a base branch
from
+179 −49
Draft

feat: Improve Admin Settings UX and restore inline comments (#7603) #7666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
caa1bc4
admin: improve settings editor UX and restore comments #7603
AyushiGupta160604 May 3, 2026
868e607
admin: implement test json dry-run and address all code review feedback
AyushiGupta160604 May 4, 2026
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
20 changes: 20 additions & 0 deletions admin/src/App.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* Enhanced Admin Settings Editor Styles */
.settings {
font-family: "Fira Code", "Cascadia Code", "Source Code Pro", monospace;
font-size: 14px;
white-space: pre;
overflow-wrap: normal;
overflow-x: auto;
}

/* VS Code style focus ring requested by John */
.settings:focus {
outline: 2px solid #007acc !important;
outline-offset: -1px;
}

.settings-button-bar {
display: flex;
gap: 10px;
margin-top: 15px;
}
208 changes: 159 additions & 49 deletions admin/src/pages/SettingsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,50 +1,160 @@
import {useStore} from "../store/store.ts";
import {isJSONClean, cleanComments} from "../utils/utils.ts";
import {Trans} from "react-i18next";
import {IconButton} from "../components/IconButton.tsx";
import {RotateCw, Save} from "lucide-react";

export const SettingsPage = ()=>{
const settingsSocket = useStore(state=>state.settingsSocket)
const settings = cleanComments(useStore(state=>state.settings))

return <div className="settings-page">
<h1><Trans i18nKey="admin_settings.current"/></h1>
<textarea value={settings} className="settings" onChange={v => {
useStore.getState().setSettings(v.target.value)
}}/>
<div className="settings-button-bar">
<IconButton className="settingsButton" icon={<Save/>}
title={<Trans i18nKey="admin_settings.current_save.value"/>} onClick={() => {
if (isJSONClean(settings!)) {
// JSON is clean so emit it to the server
settingsSocket!.emit('saveSettings', settings!);
useStore.getState().setToastState({
open: true,
title: "Successfully saved settings",
success: true
})
} else {
useStore.getState().setToastState({
open: true,
title: "Error saving settings",
success: false
})
}
}}/>
<IconButton className="settingsButton" icon={<RotateCw/>}
title={<Trans i18nKey="admin_settings.current_restart.value"/>} onClick={() => {
settingsSocket!.emit('restartServer');
}}/>
</div>
<div className="separator"/>
<div className="settings-button-bar">
<a rel="noopener noreferrer" target="_blank"
href="https://github.com/ether/etherpad-lite/wiki/Example-Production-Settings.JSON"><Trans
i18nKey="admin_settings.current_example-prod"/></a>
<a rel="noopener noreferrer" target="_blank"
href="https://github.com/ether/etherpad-lite/wiki/Example-Development-Settings.JSON"><Trans
i18nKey="admin_settings.current_example-devel"/></a>
import React, { useState } from 'react';
import { useStore } from "../store/store.ts";
import { isJSONClean, cleanComments } from "../utils/utils.ts";
import { Trans } from "react-i18next";
import { IconButton } from "../components/IconButton.tsx";
import { RotateCw, Save, AlignLeft, ShieldCheck } from "lucide-react";

export const SettingsPage = () => {
const settingsSocket = useStore(state => state.settingsSocket);

// FIX: Initialize with empty string to prevent uncontrolled->controlled warning
const settings = useStore(state => state.settings) ?? "";

// FIX: New features disabled by default per project maintenance rules
const [exposeExperimental] = useState(false);

const handleKeyDown = (e: React.KeyboardEvent<HTMLTextAreaElement>) => {
if (e.key === 'Tab') {
e.preventDefault();
const { selectionStart, selectionEnd, value } = e.currentTarget;
const newValue = value.substring(0, selectionStart) + " " + value.substring(selectionEnd);
useStore.getState().setSettings(newValue);

// Maintain cursor position after state update
setTimeout(() => {
e.currentTarget.selectionStart = e.currentTarget.selectionEnd = selectionStart + 2;
}, 0);
}
};

// Dry-run validation without saving
const testJSON = () => {
try {
const cleaned = cleanComments(settings);
JSON.parse(cleaned ?? "");
useStore.getState().setToastState({
open: true,
title: "Validation Success: JSON is structurally sound.",
success: true
});
} catch (e) {
useStore.getState().setToastState({
open: true,
title: "Validation Failed: Check for syntax errors or stray characters.",
success: false
});
}
};

const prettifyJSON = () => {
try {
const cleaned = cleanComments(settings);
const obj = JSON.parse(cleaned ?? "");
const formatted = JSON.stringify(obj, null, 2);

if (window.confirm("Prettifying will remove all comments. Do you wish to proceed?")) {
useStore.getState().setSettings(formatted);
}
} catch (e) {
useStore.getState().setToastState({
open: true,
title: "Cannot prettify: Please fix syntax errors first.",
success: false
});
}
};

return (
<div className="settings-page">
<h1><Trans i18nKey="admin_settings.current" /></h1>

<textarea
value={settings}
className="settings"
spellCheck={false}
onKeyDown={handleKeyDown}
onChange={v => useStore.getState().setSettings(v.target.value)}
style={{
fontFamily: '"Fira Code", "Cascadia Code", monospace',
width: '100%',
height: '500px',
padding: '15px',
backgroundColor: '#1e1e1e',
color: '#d4d4d4',
lineHeight: '1.5',
border: '1px solid #333',
resize: 'vertical'
}}
/>
Comment on lines +72 to +89
Copy link
Copy Markdown

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot May 4, 2026

Choose a reason for hiding this comment

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

Action required

1. Settings textarea not pretty-printed 📎 Requirement gap ≡ Correctness

The new /admin/settings UI still renders the raw settings string directly in a <textarea>
without parsing and pretty-printing it for a consistently formatted display. This fails the
requirement to present settings.json as a parsed/pretty-printed, well-formatted representation.
Agent Prompt 
## Issue description
The `/admin/settings` page still displays the raw `settings` text directly, rather than presenting a parsed/pretty-printed representation as required.

## Issue Context
Compliance requires the admin UI to show a consistently formatted, parsed/pretty-printed view that preserves readable structure.

## Fix Focus Areas
- admin/src/pages/SettingsPage.tsx[72-89]

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

Comment on lines +72 to +89
Copy link
Copy Markdown

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot May 4, 2026

Choose a reason for hiding this comment

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

Action required

2. No inline comment mapping 📎 Requirement gap ≡ Correctness

The settings UI does not extract and associate settings.json comments with their related settings
as inline annotations; it only shows a raw text blob. This does not meet the requirement to surface
comments in a setting-associated way.
Agent Prompt 
## Issue description
The admin settings page does not implement any extraction/mapping of comments to the corresponding settings for inline annotations/tooltips.

## Issue Context
Compliance requires surfacing documentation/comments originally present in `settings.json` in a way that is associated with the relevant setting (not just raw text).

## Fix Focus Areas
- admin/src/pages/SettingsPage.tsx[72-89]

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


<div className="settings-button-bar">
<IconButton
className="settingsButton"
icon={<Save />}
title={<Trans i18nKey="admin_settings.current_save.value" />}
onClick={() => {
// FIX: Separate validation logic from socket logic
if (!isJSONClean(settings)) {
useStore.getState().setToastState({
open: true, title: "Syntax Error: Check commas and braces.", success: false
});
return;
}
if (!settingsSocket?.connected) {
useStore.getState().setToastState({
open: true, title: "Error: Not connected to server.", success: false
});
return;
}
settingsSocket.emit('saveSettings', settings);
useStore.getState().setToastState({
open: true, title: "Settings saved successfully.", success: true
});
}}
/>

{/* Dry-run Button */}
<IconButton
className="settingsButton"
icon={<ShieldCheck />}
title="Test JSON (Dry-run)"
onClick={testJSON}
/>

{/* FIX: Feature Flag Gating */}
{exposeExperimental && (
<IconButton
className="settingsButton"
icon={<AlignLeft />}
title="Prettify JSON"
onClick={prettifyJSON}
/>
)}

<IconButton
className="settingsButton"
icon={<RotateCw />}
// FIX: Stable ID for Playwright automation
data-testid="restart-etherpad-button"
title={<Trans i18nKey="admin_settings.current_restart.value" />}
onClick={() => {
settingsSocket?.emit('restartServer');
}}
/>
Comment on lines +135 to +144
Copy link
Copy Markdown

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot May 4, 2026

Choose a reason for hiding this comment

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

Action required

3. Invalid iconbutton testid prop 🐞 Bug ≡ Correctness

SettingsPage passes data-testid to IconButton, but IconButtonProps does not include it and
IconButton does not forward unknown props to the underlying <button>, causing a TS build failure
and preventing the attribute from being rendered. This also blocks the intended “stable selector”
for E2E tests.
Agent Prompt 
### Issue description
`IconButton` is a wrapper around `<button>`, but it does not accept/forward standard button attributes like `data-testid`. `SettingsPage` now passes `data-testid`, which will fail TS type-checking and will not render into the DOM.

### Issue Context
The PR added `data-testid="restart-etherpad-button"` to stabilize Playwright selectors, but the wrapper component prevents it from working.

### Fix Focus Areas
- admin/src/components/IconButton.tsx[1-17]
- admin/src/pages/SettingsPage.tsx[135-144]

### Implementation notes
- Update `IconButtonProps` to extend `React.ButtonHTMLAttributes<HTMLButtonElement>` (or at least include an index signature / explicit `'data-testid'?: string`).
- Spread remaining props onto the `<button>` so attributes like `data-testid`, `aria-*`, etc. are preserved.

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

</div>

<div className="separator" style={{ margin: '20px 0', borderBottom: '1px solid #eee' }} />

<div className="settings-links" style={{ display: 'flex', gap: '20px' }}>
{/* FIX: Protocol-independent URLs */}
<a rel="noopener noreferrer" target="_blank" href="//github.com/ether/etherpad-lite/wiki/Example-Production-Settings.JSON">
<Trans i18nKey="admin_settings.current_example-prod" />
</a>
<a rel="noopener noreferrer" target="_blank" href="//github.com/ether/etherpad-lite/wiki/Example-Development-Settings.JSON">
<Trans i18nKey="admin_settings.current_example-devel" />
</a>
</div>
</div>
</div>
}
);
};