Skip to content

chore: fixed admin design rework#7716

Merged
SamTV12345 merged 2 commits intodevelopfrom
feature/admin-design-rework
May 10, 2026
Merged

chore: fixed admin design rework#7716
SamTV12345 merged 2 commits intodevelopfrom
feature/admin-design-rework

Conversation

@SamTV12345
Copy link
Copy Markdown
Member

No description provided.

@SamTV12345 SamTV12345 marked this pull request as ready for review May 10, 2026 15:42
@qodo-code-review
Copy link
Copy Markdown

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Admin interface design rework with new color system and component library

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Complete admin UI redesign with modern forest-green color system
• Rebuilt sidebar with collapsible navigation and improved visual hierarchy
• Redesigned plugin manager with stats cards, better search/filtering
• Overhauled pads management page with bulk actions and filtering
• Restructured help page with version info, hooks browser, and diagnostics
Diagram
flowchart LR
  A["Old Admin UI<br/>Basic styling"] -->|"Redesign"| B["New Design System<br/>Forest-green palette"]
  B --> C["Sidebar<br/>Collapsible nav"]
  B --> D["Plugin Manager<br/>Stats + tables"]
  B --> E["Pads Manager<br/>Bulk actions"]
  B --> F["Help Page<br/>Diagnostics"]
  C --> G["Modern UX<br/>Improved usability"]
  D --> G
  E --> G
  F --> G
Loading

Grey Divider

File Changes

1. admin/src/pages/Plugin.ts ✨ Enhancement +4/-2

Plugin type definitions extended with downloads and description

• Added optional downloads field to PluginDef type for popularity tracking
• Added optional description field to InstalledPlugin type
• Fixed spacing in version field declaration
• Extended SearchParams.sortBy to include 'downloads' sort option

admin/src/pages/Plugin.ts


2. admin/src/index.css ✨ Enhancement +1028/-82

Complete CSS redesign with new design system and component styles

• Introduced comprehensive design system with CSS variables (accent colors, forest palette, ink
 levels, spacing)
• Completely redesigned sidebar with new .sidebar-* classes for modern collapsible navigation
• Added 900+ lines of new plugin manager styles (.pm-* classes) for pages, buttons, tables, cards
• Redesigned help page, pads page, and plugin manager layouts with improved visual hierarchy
• Updated media queries for responsive design at 800px and 1100px breakpoints

admin/src/index.css


3. admin/src/App.tsx ✨ Enhancement +103/-62

Sidebar refactored with new semantic structure and collapsible design

• Restructured sidebar markup with new semantic HTML structure using .sidebar-top, .sidebar-nav,
 .sidebar-footer classes
• Converted navigation from <ul>/<li> to semantic <nav> with NavLink components
• Implemented collapsible sidebar with icon-only mode at 64px width
• Added version display in sidebar footer from updateStatus
• Improved code formatting and removed redundant comments

admin/src/App.tsx


View more (4)
4. admin/src/pages/HelpPage.tsx ✨ Enhancement +213/-58

Help page redesigned with diagnostics, tabs, and modern styling

• Complete page redesign with new layout using .pm-* component classes
• Added version hero block with update status indicator
• Implemented server/client hook tabs with search filtering
• Added diagnostic copy button and formatted metadata display
• Converted from simple lists to styled cards and pills for plugins/parts

admin/src/pages/HelpPage.tsx


5. admin/src/pages/HomePage.tsx ✨ Enhancement +313/-238

Plugin manager page redesigned with stats, cards, and improved UX

• Redesigned plugin manager with stats cards showing installed/available/updatable counts
• Implemented new search and sort UI with dropdown selector
• Converted installed plugins to card-based layout with icons and tags
• Redesigned available plugins table with downloads column and popularity badges
• Added helper functions for download formatting and improved sorting logic

admin/src/pages/HomePage.tsx


6. admin/src/pages/PadPage.tsx ✨ Enhancement +377/-240

Pads manager redesigned with filters, bulk actions, and modern table

• Complete pads management redesign with stats cards and filter chips
• Implemented bulk selection with checkbox controls and bulk action bar
• Added filter options (all, active, recent, empty, stale) with helper functions
• Redesigned table with improved columns and row actions
• Added relative time formatting and pagination controls

admin/src/pages/PadPage.tsx


7. admin/package.json ✨ Enhancement +1/-0

Added dev:only npm script for faster development

• Added new dev:only script that runs Vite without API generation

admin/package.json


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 10, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Hardcoded https://www.npmjs.com ✓ Resolved 📘 Rule violation ⛨ Security
Description
The new admin UI link uses a protocol-specific external URL (https://...) instead of a
protocol-independent URL. This violates the checklist requirement to use protocol-independent URLs
where applicable to avoid protocol-coupling issues.
Code

admin/src/pages/HomePage.tsx[R138-145]

+          <a
+            className="pm-btn pm-btn-primary pm-btn-link"
+            href={`https://www.npmjs.com/search?q=${encodeURIComponent(searchTerm ? `keywords:etherpad ${searchTerm}` : 'keywords:etherpad')}`}
+            target="_blank"
+            rel="noreferrer"
+          >
+            <ExternalLink size={14}/> Auf npm suchen
+          </a>
Evidence
PR Compliance ID 9 requires protocol-independent URLs for external resources where applicable. The
added npm search link hard-codes https://, which fails the rule's success criteria.

admin/src/pages/HomePage.tsx[138-145]
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
A new external link hard-codes `https://` (`https://www.npmjs.com/...`) instead of using a protocol-independent URL as required by the compliance checklist.
## Issue Context
This URL is used for the npm search link in the admin plugins page.
## Fix Focus Areas
- admin/src/pages/HomePage.tsx[138-145]

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


2. Plugin listeners not cleaned ✓ Resolved 🐞 Bug ☼ Reliability
Description
HomePage registers multiple Socket.IO handlers inside effects that re-run (e.g., when searchParams
or the socket instance changes) but only clears an interval on unmount, so listeners are never
detached. This allows handlers to fire after unmount and to accumulate over time, causing duplicate
state updates and unbounded listener growth.
Code

admin/src/pages/HomePage.tsx[R58-89]

+  useEffect(() => {
+    if (!pluginsSocket) return
-        // check for updates every 5mins
-        const interval = setInterval(() => {
-            pluginsSocket.emit('checkUpdates');
-        }, 1000 * 60 * 5);
-
-        return ()=>{
-            clearInterval(interval)
-        }
-        }, [pluginsSocket]);
+    pluginsSocket.on('results:installed', (data: {installed: InstalledPlugin[]}) => {
+      setInstalledPlugins(data.installed)
+    })
+    pluginsSocket.on('results:updatable', (data) => {
+      const updated = useStore.getState().installedPlugins.map(plugin =>
+        data.updatable.includes(plugin.name) ? {...plugin, updatable: true} : plugin
+      )
+      setInstalledPlugins(updated)
+    })
-    useEffect(() => {
-        if (!pluginsSocket) {
-            return
-        }
-        pluginsSocket?.emit('search', searchParams)
-        pluginsSocket!.on('results:search', (data: {
-            results: PluginDef[]
-        }) => {
-            setPlugins(data.results)
-        })
-        pluginsSocket!.on('results:searcherror', (data: {error: string}) => {
-            console.log(data.error)
-            useStore.getState().setToastState({
-                open: true,
-                title: "Error retrieving plugins",
-                success: false
-            })
-        })
-    }, [searchParams, pluginsSocket]);
+    pluginsSocket.on('finished:install', () => {
+      pluginsSocket.emit('getInstalled')
+    })
-    const uninstallPlugin  = (pluginName: string)=>{
-        pluginsSocket!.emit('uninstall', pluginName);
-        // Remove plugin
-        setInstalledPlugins(installedPlugins.filter(i=>i.name !== pluginName))
-    }
+    pluginsSocket.on('finished:uninstall', () => {
+      console.log('Finished uninstall')
+    })
-    const installPlugin = (pluginName: string)=>{
-        pluginsSocket!.emit('install', pluginName);
-        setPlugins(plugins.filter(plugin=>plugin.name !== pluginName))
-    }
+    pluginsSocket.on('connect', () => {
+      pluginsSocket.emit('getInstalled')
+      pluginsSocket.emit('search', searchParams)
+    })
-    useDebounce(()=>{
-        setSearchParams({
-            ...searchParams,
-            offset: 0,
-            searchTerm: searchTerm
-        })
-    }, 500, [searchTerm])
+    pluginsSocket.emit('getInstalled')
+    const interval = setInterval(() => pluginsSocket.emit('checkUpdates'), 1000 * 60 * 5)
+    return () => clearInterval(interval)
+  }, [pluginsSocket])
Evidence
The cited HomePage effects attach several pluginsSocket.on(...) listeners (including results:*,
finished:*, and connect), and one of those effects is keyed on searchParams, meaning it will
re-run on debounced search/sort changes and re-register handlers. However, the corresponding effect
cleanup only clears the setInterval and does not call pluginsSocket.off(...) for any event, so
previously registered listeners persist across re-renders and navigation, leading to duplicated
handling and potential memory leaks.

admin/src/pages/HomePage.tsx[58-89]
admin/src/pages/HomePage.tsx[91-100]

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

## Issue description
`HomePage` attaches Socket.IO listeners (e.g., `results:installed`, `results:updatable`, `results:search`, `results:searcherror`, `finished:*`, `connect`) inside React effects, including effects that re-run when `searchParams` changes, but it does not unregister those listeners in cleanup. Update the component so every registered `.on()` handler is removed appropriately to prevent duplicate event handling, unbounded listener growth, and handlers firing after unmount.
## Issue Context
The current effect cleanup only clears a `setInterval`, leaving all Socket.IO `.on()` handlers attached. Because Socket.IO listeners persist until explicitly removed, any effect that re-runs (such as one dependent on `searchParams`, or one that might re-run if the socket instance changes) will accumulate additional listeners unless `off(event, handler)` is called, leading to multiple state updates per single server response.
## Fix Focus Areas
- admin/src/pages/HomePage.tsx[58-89]
- admin/src/pages/HomePage.tsx[91-100]

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


3. Pad listeners multiply 🐞 Bug ☼ Reliability
Description
PadPage’s socket listener effect depends on pads and also updates pads inside listeners, so each
update re-runs the effect and registers additional handlers, multiplying events over time.
Code

admin/src/pages/PadPage.tsx[R105-134]

useEffect(() => {
-    if(!settingsSocket){
-      return
-    }
+    if (!settingsSocket) return
-    settingsSocket.on('results:padLoad', (data: PadSearchResult)=>{
-      useStore.getState().setPads(data);
+    settingsSocket.on('results:padLoad', (data: PadSearchResult) => {
+      useStore.getState().setPads(data)
  })
+    settingsSocket.on('results:deletePad', (padID: string) => {
+      const newPads = useStore.getState().pads?.results?.filter(p => p.padName !== padID)
+      useStore.getState().setPads({total: useStore.getState().pads!.total - 1, results: newPads})
+    })
-    settingsSocket.on('results:deletePad', (padID: string)=>{
-      const newPads = useStore.getState().pads?.results?.filter((pad)=>{
-        return pad.padName !== padID
-      })
-      useStore.getState().setPads({
-        total: useStore.getState().pads!.total-1,
-        results: newPads
-      })
+    type CreateResponse = {error: string} | {success: string}
+    settingsSocket.on('results:createPad', (rep: CreateResponse) => {
+      if ('error' in rep) {
+        useStore.getState().setToastState({open: true, title: rep.error, success: false})
+      } else {
+        useStore.getState().setToastState({open: true, title: rep.success, success: true})
+        setCreatePadDialogOpen(false)
+        settingsSocket.emit('padLoad', searchParams)
+      }
  })
-   type SettingsSocketCreateReponse = {
-     error: string
-   } | {
-    success: string
-   }
+    settingsSocket.on('results:cleanupPadRevisions', (data) => {
+      const newPads = useStore.getState().pads?.results ?? []
+      if (data.error) { setErrorText(data.error); return }
+      newPads.forEach(p => { if (p.padName === data.padId) p.revisionNumber = data.keepRevisions })
+      useStore.getState().setPads({results: newPads, total: useStore.getState().pads!.total})
+    })
+  }, [settingsSocket, pads])
Evidence
The effect’s dependency array includes pads, and the results:padLoad handler calls setPads,
which changes pads and re-triggers the effect, adding another set of listeners; there is also no
cleanup via off(...).

admin/src/pages/PadPage.tsx[105-134]

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

## Issue description
`PadPage` registers multiple Socket.IO listeners in an effect that depends on `[settingsSocket, pads]`. Because the listeners update `pads`, the effect re-runs and re-registers listeners, causing handler duplication and progressively worse behavior.
### Issue Context
This will amplify actions like `padLoad` (multiple `setPads` calls), and also duplicates delete/create/cleanup handlers.
### Fix Focus Areas
- admin/src/pages/PadPage.tsx[105-134]
### Suggested fix
- Change the effect deps to `[settingsSocket]` (or `[settingsSocket, searchParams]` only if truly needed) and **remove `pads`**.
- Add cleanup that calls `settingsSocket.off(...)` for each registered event.
- Avoid mutating `useStore.getState().pads?.results` in place in `results:cleanupPadRevisions`; instead create a new array (e.g., `map`) and new objects before calling `setPads`.

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



Remediation recommended

4. Undefined CSS vars used 🐞 Bug ≡ Correctness
Description
The new sidebar/pads styles reference --line-1 and --ink-1, but these variables are not defined
in :root, causing affected border/color declarations to be dropped by the browser.
Code

admin/src/index.css[R1524-1529]

+.pm-chip {
+  height: 28px;
+  padding: 0 12px;
+  border-radius: 14px;
+  border: 1px solid var(--line-1);
+  background: transparent;
Evidence
:root defines --line/--line-2 and --ink/--ink-2..4, but later rules use var(--line-1)
and var(--ink-1) with no fallback, making those properties invalid.

admin/src/index.css[2-27]
admin/src/index.css[1524-1531]
admin/src/index.css[1561-1577]
admin/src/index.css[1614-1618]

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

## Issue description
`admin/src/index.css` uses CSS variables `--line-1` and `--ink-1` that are not defined in `:root`. CSS `var()` without a fallback invalidates the entire declaration, leading to missing borders and incorrect text/icon colors.
### Issue Context
The design system defines `--line` and `--ink` (and numbered variants starting at `-2`), but the new rules reference `-1` variants.
### Fix Focus Areas
- admin/src/index.css[2-27]
- admin/src/index.css[1524-1531]
- admin/src/index.css[1561-1577]
- admin/src/index.css[1614-1618]
### Suggested fix
Choose one:
1. **Define aliases** in `:root`, e.g.:
- `--line-1: var(--line);`
- `--ink-1: var(--ink);`
2. **Replace usages**:
- Replace `var(--line-1)` with `var(--line)`
- Replace `var(--ink-1)` with `var(--ink)`
3. Add fallbacks in `var()` if you want resilience, e.g. `var(--line-1, var(--line))`.

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


5. Help listener not removed 🐞 Bug ☼ Reliability
Description
HelpPage registers the reply:help Socket.IO handler but never removes it, so remounting the page
can accumulate listeners and trigger repeated setHelpData calls.
Code

admin/src/pages/HelpPage.tsx[R14-18]

useEffect(() => {
-    if(!settingsSocket) return;
-    settingsSocket?.on('reply:help', (data) => {
-      setHelpData(data)
-    });
-
-    settingsSocket?.emit('help');
-  }, [settingsSocket]);
-
-  const renderHooks = (hooks:Record<string, Record<string, string>>) => {
-    return Object.keys(hooks).map((hookName, i) => {
-      return <div key={hookName+i}>
-        <h3>{hookName}</h3>
-        <ul>
-          {Object.keys(hooks[hookName]).map((hook, i) => <li key={hook+i}>{hook}
-            <ul key={hookName+hook+i}>
-              {Object.keys(hooks[hookName][hook]).map((subHook, i) => <li key={i}>{subHook}</li>)}
-            </ul>
-          </li>)}
-        </ul>
-      </div>
-    })
+    if (!settingsSocket) return
+    settingsSocket.on('reply:help', (data) => setHelpData(data))
+    settingsSocket.emit('help')
+  }, [settingsSocket])
Evidence
The effect adds a persistent settingsSocket.on('reply:help', ...) listener and emits help, but
provides no cleanup to detach the listener on unmount or socket change.

admin/src/pages/HelpPage.tsx[14-18]

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

## Issue description
`HelpPage` attaches a `reply:help` socket listener without removing it. If the component unmounts/remounts (route changes) or the socket instance is replaced, listeners can accumulate.
### Issue Context
Socket.IO listeners persist unless removed via `.off()`.
### Fix Focus Areas
- admin/src/pages/HelpPage.tsx[14-18]
### Suggested fix
- Create a named handler `const onHelp = (data) => setHelpData(data)`.
- Register with `settingsSocket.on('reply:help', onHelp)`.
- Return cleanup: `() => settingsSocket.off('reply:help', onHelp)`.

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


Grey Divider

Qodo Logo

Comment thread admin/src/pages/HomePage.tsx
Comment thread admin/src/pages/HomePage.tsx
Comment on lines 105 to +134
useEffect(() => {
if(!settingsSocket){
return
}
if (!settingsSocket) return

settingsSocket.on('results:padLoad', (data: PadSearchResult)=>{
useStore.getState().setPads(data);
settingsSocket.on('results:padLoad', (data: PadSearchResult) => {
useStore.getState().setPads(data)
})

settingsSocket.on('results:deletePad', (padID: string) => {
const newPads = useStore.getState().pads?.results?.filter(p => p.padName !== padID)
useStore.getState().setPads({total: useStore.getState().pads!.total - 1, results: newPads})
})

settingsSocket.on('results:deletePad', (padID: string)=>{
const newPads = useStore.getState().pads?.results?.filter((pad)=>{
return pad.padName !== padID
})
useStore.getState().setPads({
total: useStore.getState().pads!.total-1,
results: newPads
})
type CreateResponse = {error: string} | {success: string}
settingsSocket.on('results:createPad', (rep: CreateResponse) => {
if ('error' in rep) {
useStore.getState().setToastState({open: true, title: rep.error, success: false})
} else {
useStore.getState().setToastState({open: true, title: rep.success, success: true})
setCreatePadDialogOpen(false)
settingsSocket.emit('padLoad', searchParams)
}
})

type SettingsSocketCreateReponse = {
error: string
} | {
success: string
}
settingsSocket.on('results:cleanupPadRevisions', (data) => {
const newPads = useStore.getState().pads?.results ?? []
if (data.error) { setErrorText(data.error); return }
newPads.forEach(p => { if (p.padName === data.padId) p.revisionNumber = data.keepRevisions })
useStore.getState().setPads({results: newPads, total: useStore.getState().pads!.total})
})
}, [settingsSocket, pads])
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

3. Pad listeners multiply 🐞 Bug ☼ Reliability

PadPage’s socket listener effect depends on pads and also updates pads inside listeners, so each
update re-runs the effect and registers additional handlers, multiplying events over time.
Agent Prompt
### Issue description
`PadPage` registers multiple Socket.IO listeners in an effect that depends on `[settingsSocket, pads]`. Because the listeners update `pads`, the effect re-runs and re-registers listeners, causing handler duplication and progressively worse behavior.

### Issue Context
This will amplify actions like `padLoad` (multiple `setPads` calls), and also duplicates delete/create/cleanup handlers.

### Fix Focus Areas
- admin/src/pages/PadPage.tsx[105-134]

### Suggested fix
- Change the effect deps to `[settingsSocket]` (or `[settingsSocket, searchParams]` only if truly needed) and **remove `pads`**.
- Add cleanup that calls `settingsSocket.off(...)` for each registered event.
- Avoid mutating `useStore.getState().pads?.results` in place in `results:cleanupPadRevisions`; instead create a new array (e.g., `map`) and new objects before calling `setPads`.

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

@SamTV12345 SamTV12345 merged commit 41d10ec into develop May 10, 2026
41 of 42 checks passed
@SamTV12345 SamTV12345 deleted the feature/admin-design-rework branch May 10, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant