-
-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(skin): add margin skin (11 themes; light/dark per theme; switchable from Settings)#7880
Closed
deepshekhardas wants to merge 1 commit into
Closed
feat(skin): add margin skin (11 themes; light/dark per theme; switchable from Settings) #7880deepshekhardas wants to merge 1 commit into
deepshekhardas wants to merge 1 commit into
Conversation
...ble from Settings) Upstream PR: ether#7721
qodo-code-review
Bot
commented
Jun 2, 2026
•
edited by qodo-free-for-open-source-projects
Bot
Loading
edited by qodo-free-for-open-source-projects
Bot
Review Summary by Qodo
(Agentic_describe updated until commit 25a7158)
Add margin skin with 6 themes and light/dark mode support
✨ Enhancement
Walkthroughs
Description
• Adds a new margin skin with 6 selectable themes (Colibris, Editorial, Brutalist, Paper, CRT, Industrial) and light/dark mode toggle • Implements theme and dark mode management across pad, lobby, and timeslider pages with early application to prevent flash of unstyled content • Provides complete CSS variable system for all 11 theme combinations with theme-specific visual touches (italic styling, brutalist borders, paper rounded corners, CRT scanlines and glow effects, Industrial accent underlines) • Builds theme selector dropdown and dark mode toggle UI elements injected into user-settings and pad-settings sections • Propagates theme attributes into nested editor iframes (ace_outer and ace_inner) for visual consistency • Implements recent pads list functionality on lobby page with deduplication, sorting by timestamp, and formatted display with member counts • Includes comprehensive styling for all UI components (toolbar, buttons, forms, popups, chat, comments, tables2 plugin, color picker, etc.) with theme-aware colors • Adds scrollbar, layout, and responsive design support for various viewport sizes • Includes complete documentation for installation, configuration, and runtime theme switching
Diagram
flowchart LR
A["User Settings"] -->|"Select Theme"| B["applyMarginTheme()"]
A -->|"Toggle Dark Mode"| C["applyMarginMode()"]
B -->|"Set data-theme"| D["Document Root"]
C -->|"Set data-mode"| D
D -->|"Apply CSS Variables"| E["Pad/Lobby/Timeslider"]
D -->|"Propagate to iframes"| F["Editor Iframes"]
G["localStorage"] -->|"Restore Preferences"| B
G -->|"Restore Preferences"| C
H["Recent Pads List"] -->|"Display with Theme"| I["Lobby Page"]
File Changes
1. src/static/skins/margin/pad.js
✨ Enhancement +253/-0
Theme and dark mode management for margin skin
• Implements theme and dark mode management for the margin skin with 6 selectable themes (Colibris, Editorial, Brutalist, Paper, CRT, Industrial) • Provides applyMarginTheme() and applyMarginMode() functions to set data-theme and data-mode attributes on the document root • Builds theme selector dropdown and dark mode toggle UI elements that are injected into both user-settings and pad-settings sections • Propagates theme attributes into nested editor iframes (ace_outer and ace_inner) and maintains a recent pads list in localStorage
2. src/static/skins/margin/index.js
✨ Enhancement +173/-0
Lobby page theme application and recent pads display
• Applies saved theme and light/dark mode preferences early on page load to prevent flash of unstyled content • Implements recent pads list functionality with deduplication, sorting by timestamp, and display with formatted dates and member counts • Renders recent pad entries as clickable list items with SVG icons for navigation, timestamps, and user counts • Handles localStorage access gracefully with fallbacks for private mode and corrupted data
3. src/static/skins/margin/timeslider.js
✨ Enhancement +42/-0
Timeslider page theme initialization
• Bootstraps theme and dark mode for the timeslider (history view) page by reading from parent document or localStorage • Ensures history iframe renders with the same theme as the parent pad for visual consistency • Validates theme values against allowed list and applies sensible defaults for each theme's natural mode
(追記) View more (25) (追記ここまで)
4. src/static/skins/margin/pad.css
✨ Enhancement +416/-0
Complete theme and component styling system
• Defines complete CSS variable system for 6 themes with light/dark variants (11 total theme combinations) • Imports Google Fonts and vendored component stylesheets from colibris for standalone skin functionality • Applies theme-specific styling to toolbar, popups, buttons, forms, and editor frames using [data-theme] and [data-mode] selectors • Includes per-theme visual touches like italic styling for Editorial, brutalist borders, paper rounded corners, CRT scanlines and glow effects, and Industrial accent underlines
5. src/static/skins/margin/index.css
✨ Enhancement +43/-0
Lobby page theme styling
• Defines theme color variables for the lobby/index page mirroring pad.css themes • Styles page elements (body, nav, logo, wrapper, buttons, recent pads list) using theme variables • Applies theme-specific styling to brutalist variant with 2px borders and shadow effects
6. src/static/skins/margin/src/pad-variants.css
✨ Enhancement +228/-0
Pad layout and brightness variant styles
• Provides CSS classes for toolbar, background, and editor brightness variants (super-light, light, super-dark, dark) • Allows independent control of toolbar and editor background colors through class combinations • Includes full-width editor mode and responsive adjustments for narrow viewports
7. src/static/skins/margin/src/plugins/tables2.css
✨ Enhancement +239/-0
Tables2 plugin styling
• Styles the tables2 plugin UI including menu icons, dropdown menus, table size picker, and settings popup • Customizes YUI panel containers, buttons, color pickers, and table cell styling • Applies modern shadows, border-radius, and spacing to table editing controls
8. src/static/skins/margin/src/components/popup.css
✨ Enhancement +177/-0
Popup and settings modal styling
• Styles popup containers, settings sections, and form elements with theme-aware colors • Implements responsive grid layout for settings sections on wider viewports • Styles deletion token modal, pad deletion controls, and comment modals
9. src/static/skins/margin/src/components/toolbar.css
✨ Enhancement +154/-0
Toolbar and button styling
• Styles the main toolbar with theme-aware colors, hover states, and button styling • Implements nice-select widget styling for dropdowns in the toolbar • Handles responsive behavior for mobile layouts and condensed toolbar modes
10. src/static/skins/margin/src/components/form.css
✨ Enhancement +122/-0
Form controls and checkbox styling
• Styles form inputs (text, select, textarea) with theme-aware colors and consistent padding • Implements custom checkbox styling with animated toggle effect using CSS pseudo-elements • Applies disabled state styling for form controls
11. src/static/skins/margin/src/plugins/comments.css
✨ Enhancement +112/-0
Comments plugin styling
• Styles comment sidebar, comment modals, and suggestion displays with theme-aware colors • Customizes comment highlights in the editor with tinted underlines instead of solid backgrounds • Applies responsive width adjustments for wider viewports
12. src/static/skins/margin/src/pad-editor.css
✨ Enhancement +48/-0
Editor iframe theme and comment styling
• Defines theme tokens for the inner editor iframe (innerdocbody) since CSS doesn't cascade into iframes • Implements theme-aware comment highlighting with color-mix and box-shadow effects • Adds CRT theme drop-cap styling with glow effects and special font rendering
13. src/static/skins/margin/timeslider.css
✨ Enhancement +108/-0
Timeslider history view styling
• Styles the timeslider (history view) UI including slider handle, buttons, and author list • Applies theme-aware colors to play/pause button, stepper controls, and timer display • Includes responsive adjustments for mobile viewports
14. src/static/skins/margin/src/components/gritter.css
✨ Enhancement +82/-0
Notification popup styling
• Styles notification popups (gritter items) with theme-aware colors and shadows • Implements separate styling for chat notifications vs regular notifications • Adds animation transforms with reduced-motion media query support
15. src/static/skins/margin/src/components/chat.css
✨ Enhancement +91/-0
Chat component styling
• Styles the chat box and chat icon with theme-aware colors and shadows • Implements sticky chat mode styling and responsive adjustments for mobile • Customizes chat input, title bar, and message text styling
16. src/static/skins/margin/src/components/buttons.css
✨ Enhancement +74/-0
Button component styling
• Defines button styles for primary, default, secondary, and danger variants • Implements hover and disabled states with transforms and shadows • Applies theme-aware colors using CSS variables
17. src/static/skins/margin/src/components/users.css
✨ Enhancement +65/-0
Users panel styling
• Styles the users table and color picker with theme-aware colors • Implements hover effects on user names and custom styling for user swatches • Customizes the username edit form with bottom border styling
18. src/static/skins/margin/src/layout.css
✨ Enhancement +48/-0
Main layout structure and spacing
• Defines the main layout structure for editor container, outer document body, and side divider • Applies theme-aware colors and responsive padding adjustments • Implements responsive behavior for narrow viewports (max-width: 1000px)
19. src/static/skins/margin/src/components/scrollbars.css
✨ Enhancement +41/-0
Scrollbar styling
• Styles webkit scrollbars with theme-aware colors and rounded corners • Implements thin scrollbar variant for specific UI elements • Applies responsive styling for viewports wider than 721px
20. src/static/skins/margin/src/components/sidediv.css
✨ Enhancement +31/-0
Line number sidebar styling
• Styles the line number sidebar with theme-aware colors and hover effects • Implements font family and styling for line numbers • Adds special handling for author neat plugin compatibility
21. src/static/skins/margin/src/plugins/font_color.css
✨ Enhancement +41/-0
Font color plugin styling
• Defines color palette for text color plugin with theme-aware primary color • Hides font-color icon and shows font-color menu item • Applies specific colors for black, red, green, blue, yellow, and orange text
22. src/static/skins/margin/src/plugins/brightcolorpicker.css
✨ Enhancement +14/-0
Color picker plugin styling
• Styles the color picker cancel button and color panel with modern shadows and border-radius • Applies white background and theme-aware box-shadow to color picker
23. src/static/skins/margin/src/components/table-of-content.css
✨ Enhancement +21/-0
Table of contents sidebar styling
• Styles the table of contents sidebar with transparent background and custom padding • Implements responsive adjustments for narrow viewports and resizable bars plugin
24. src/static/skins/margin/src/plugins/author_hover.css
✨ Enhancement +10/-0
Author hover tooltip styling
• Styles author hover tooltips with uppercase text, custom padding, and semi-transparent white background • Applies consistent font sizing and weight for tooltip display
25. src/static/skins/margin/src/general.css
✨ Enhancement +11/-0
General page styling
• Defines base body and heading styles using theme-aware CSS variables • Sets default font family and text colors for the entire page
26. src/static/skins/margin/src/plugins/set_title_on_pad.css
✨ Enhancement +7/-0
Pad title styling
• Styles the pad title element with theme-aware border and background colors • Customizes the title edit input styling
27. src/static/skins/margin/src/components/import-export.css
✨ Enhancement +5/-0
Import/export component styling
• Styles import/export error messages and disabled submit button • Applies opacity reduction for disabled import submit input
28. src/static/skins/margin/README.md
📝 Documentation +61/-0
Documentation for margin skin with themes and modes
• Adds comprehensive documentation for the new margin skin with 6 themes and light/dark mode toggle • Documents installation steps and configuration in settings.json • Explains runtime theme switching via Settings popup dropdown and dark mode checkbox • Provides folder structure overview and details on CSS variable inheritance from colibris
Code Review by Qodo
🐞 Bugs (4) 📘 Rule violations (0)
1. Non-UTF8 skin files 🐞 Bug ☼ Reliability
Description
Several newly added margin skin assets contain invalid UTF-8 byte sequences (visible as mojibake like "G��" / "Sekund+�r"), which can break tooling that reads these files as UTF-8 and risks serving garbled text to browsers. This needs re-encoding/cleanup before merge to avoid brittle builds and hard-to-debug asset issues.
Evidence
The added files contain visible mojibake sequences in their content (e.g., README heading and other comments), indicating invalid/corrupted text encoding in the committed sources.
src/static/skins/margin/README.md[1-16]
src/static/skins/margin/pad.css[1-30]
src/static/skins/margin/src/components/buttons.css[24-32]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ### Issue description Multiple newly added `margin` skin files contain invalid UTF-8 bytes (mojibake sequences like `G��` / `+�`), which breaks UTF-8 decoding in tooling and can result in corrupted comments/text in served assets. ### Issue Context These files are static assets (CSS/JS/MD) that are expected to be UTF-8 in typical web/build pipelines. ### Fix Focus Areas - src/static/skins/margin/README.md[1-61] - src/static/skins/margin/index.css[1-43] - src/static/skins/margin/pad.css[1-416] - src/static/skins/margin/pad.js[1-253] - src/static/skins/margin/timeslider.js[1-42] - src/static/skins/margin/src/pad-editor.css[1-48] - src/static/skins/margin/src/components/buttons.css[1-74] ### What to change - Convert the listed files to valid UTF-8. - Replace any corrupted punctuation/characters in comments (or remove them) so the repository contains clean UTF-8 sources. - Re-run whatever formatter/linter/build step is used for static assets to ensure no further encoding regressions.
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Recent pad link broken 🐞 Bug ≡ Correctness
Description
margin/pad.js stores the pad name directly from window.location.pathname (percent-encoded), but margin/index.js applies encodeURIComponent() again when building the recent-pad URL, which double-encodes and breaks navigation for pad names containing spaces or non-ASCII characters. Users will click a recent pad and be sent to a different (wrong) pad URL (e.g., %20 becomes %2520).
Evidence
The core lobby code generates pad URLs using encodeURIComponent(), so the resulting window.location.pathname segment is percent-encoded; margin/pad.js stores that raw segment, and margin/index.js encodes it again when constructing the recent-pad URL.
src/static/skins/margin/index.js[104-113]
src/static/skins/margin/pad.js[218-242]
src/static/js/index.ts[43-52]
src/static/js/timeslider.ts[96-99]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ### Issue description Recent pad links can be double-encoded: the stored pad name is derived from `window.location.pathname` (already percent-encoded) and later encoded again with `encodeURIComponent()` when generating the link. ### Issue Context Core navigation encodes pad names when generating `/p/<pad>` URLs, so the pathname segment can contain `%..` sequences. ### Fix Focus Areas - src/static/skins/margin/pad.js[218-249] - src/static/skins/margin/index.js[104-113] ### What to change Choose ONE consistent representation for `recentPads[].name`: 1) **Store decoded names**: in `pad.js`, set `padName = decodeURIComponent(lastSegment)` before saving. Keep `encodeURIComponent(pad.name)` in `index.js`. OR 2) **Store encoded names**: keep current storage, but in `index.js` remove the extra encoding and treat `pad.name` as already encoded when building the URL. Also consider displaying the decoded name to users (so they don’t see `%20` in the list).
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3. Invalid timestamp crashes lobby 🐞 Bug ☼ Reliability
Description
margin/index.js filters recentPads entries only by name and then unconditionally formats `new Date(pad.timestamp) via toLocaleDateString(), which throws if timestamp` is missing or invalid. A single malformed entry can abort recent-pad rendering and break the lobby’s recent pads UI.
Evidence
The parsing step only checks for a name string, but later code sorts by timestamp and calls toLocaleDateString() on a Date constructed from pad.timestamp without validating that it is present/valid.
src/static/skins/margin/index.js[55-69]
src/static/skins/margin/index.js[71-75]
src/static/skins/margin/index.js[134-147]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ### Issue description `recentPads` entries are only validated for `name`, but `timestamp` is used for sorting and formatting without validation. Invalid timestamps can throw when calling `toLocaleDateString()`. ### Issue Context The code explicitly tries to tolerate corrupted `localStorage` entries, but it does not validate `timestamp`. ### Fix Focus Areas - src/static/skins/margin/index.js[60-75] - src/static/skins/margin/index.js[134-147] ### What to change - When parsing `recentPads`, additionally validate `timestamp` (and optionally `members`). - Example: keep only entries where `typeof p.timestamp === 'string'` and `!Number.isNaN(Date.parse(p.timestamp))`. - In rendering, guard formatting: - If `Date.parse()` is NaN, either skip the date, show a placeholder, or fall back to `''`. - In the sort comparator, handle invalid dates deterministically (e.g., treat invalid as oldest).
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
4. Lobby theme not validated 🐞 Bug ≡ Correctness
Description
margin/index.js applies marginTheme/marginMode from localStorage directly to <html> without validating against supported theme/mode values. Corrupted/unknown values can cause inconsistent styling compared to pad/timeslider (which do validate) and make the lobby render with missing theme tokens.
Evidence
The lobby code sets data-theme and data-mode from localStorage without checking that they are among supported values; this differs from the intended pattern elsewhere in the skin that uses allowlists/defaults.
src/static/skins/margin/index.js[7-22]
src/static/skins/margin/pad.js[34-56]
src/static/skins/margin/timeslider.js[34-39]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ### Issue description The lobby bootstrap reads `marginTheme` / `marginMode` from localStorage and sets them as attributes without allowlist validation. ### Issue Context Other pages (pad/timeslider) validate theme/mode against a known list before applying. ### Fix Focus Areas - src/static/skins/margin/index.js[7-22] ### What to change - Add a theme allowlist (same values as pad.js/timeslider.js) and validate `theme` before setting `data-theme`. - Validate `mode` to be only `light` or `dark`; if missing/invalid, fall back to the selected theme’s natural default. - Keep the existing try/catch for private mode storage failures.
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Code Review by Qodo
🐞 Bugs (6) 📘 Rule violations (0)
1. Recent pad links break 🐞 Bug ≡ Correctness ⭐ New
Description
margin/pad.js stores the pad name directly from window.location.pathname (already percent-encoded), but margin/index.js builds recent-pad URLs using encodeURIComponent(pad.name), causing double-encoding (e.g., my%20pad → my%2520pad) and broken navigation from the lobby’s recent list.
Evidence
The pad name is taken from location.pathname (percent-encoded) and persisted to recentPads, then re-encoded when building the lobby link, which produces a double-encoded path for pads containing characters that require encoding.
src/static/skins/margin/index.js[104-108]
src/static/skins/margin/pad.js[218-220]
src/static/skins/margin/pad.js[251-252]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ## Issue description Recent pad links in the lobby can become invalid because pad names are persisted in `recentPads` in an already-encoded form (from `location.pathname`), then encoded again in `index.js` when constructing the URL. ## Issue Context - `pad.js` stores the last path segment from `location.pathname` as the pad name. - Browsers keep `location.pathname` percent-encoded. - `index.js` encodes `pad.name` again via `encodeURIComponent()`. ## Fix Focus Areas - src/static/skins/margin/pad.js[218-252] - src/static/skins/margin/index.js[104-114] ## Implementation notes - When capturing the pad name in `pad.js`, safely decode it before storing (wrap `decodeURIComponent()` in try/catch). - When rendering the recent list in `index.js`, treat stored names as decoded and apply `encodeURIComponent()` exactly once when building the URL. - Consider also displaying the decoded name in the UI (so users don’t see `%20`).
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Non-UTF8 skin files 🐞 Bug ☼ Reliability
Description
Several newly added margin skin assets contain invalid UTF-8 byte sequences (visible as mojibake like "G��" / "Sekund+�r"), which can break tooling that reads these files as UTF-8 and risks serving garbled text to browsers. This needs re-encoding/cleanup before merge to avoid brittle builds and hard-to-debug asset issues.
Evidence
The added files contain visible mojibake sequences in their content (e.g., README heading and other comments), indicating invalid/corrupted text encoding in the committed sources.
src/static/skins/margin/README.md[1-16]
src/static/skins/margin/pad.css[1-30]
src/static/skins/margin/src/components/buttons.css[24-32]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ## Issue description Multiple newly added `margin` skin files contain invalid UTF-8 bytes (mojibake sequences like `G��` / `+�`), which breaks UTF-8 decoding in tooling and can result in corrupted comments/text in served assets. ### Issue Context These files are static assets (CSS/JS/MD) that are expected to be UTF-8 in typical web/build pipelines. ### Fix Focus Areas - src/static/skins/margin/README.md[1-61] - src/static/skins/margin/index.css[1-43] - src/static/skins/margin/pad.css[1-416] - src/static/skins/margin/pad.js[1-253] - src/static/skins/margin/timeslider.js[1-42] - src/static/skins/margin/src/pad-editor.css[1-48] - src/static/skins/margin/src/components/buttons.css[1-74] ### What to change - Convert the listed files to valid UTF-8. - Replace any corrupted punctuation/characters in comments (or remove them) so the repository contains clean UTF-8 sources. - Re-run whatever formatter/linter/build step is used for static assets to ensure no further encoding regressions.
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3. Recent pad link broken 🐞 Bug ≡ Correctness
Description
margin/pad.js stores the pad name directly from window.location.pathname (percent-encoded), but margin/index.js applies encodeURIComponent() again when building the recent-pad URL, which double-encodes and breaks navigation for pad names containing spaces or non-ASCII characters. Users will click a recent pad and be sent to a different (wrong) pad URL (e.g., %20 becomes %2520).
Evidence
The core lobby code generates pad URLs using encodeURIComponent(), so the resulting window.location.pathname segment is percent-encoded; margin/pad.js stores that raw segment, and margin/index.js encodes it again when constructing the recent-pad URL.
src/static/skins/margin/index.js[104-113]
src/static/skins/margin/pad.js[218-242]
src/static/js/index.ts[43-52]
src/static/js/timeslider.ts[96-99]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ## Issue description Recent pad links can be double-encoded: the stored pad name is derived from `window.location.pathname` (already percent-encoded) and later encoded again with `encodeURIComponent()` when generating the link. ### Issue Context Core navigation encodes pad names when generating `/p/<pad>` URLs, so the pathname segment can contain `%..` sequences. ### Fix Focus Areas - src/static/skins/margin/pad.js[218-249] - src/static/skins/margin/index.js[104-113] ### What to change Choose ONE consistent representation for `recentPads[].name`: 1) **Store decoded names**: in `pad.js`, set `padName = decodeURIComponent(lastSegment)` before saving. Keep `encodeURIComponent(pad.name)` in `index.js`. OR 2) **Store encoded names**: keep current storage, but in `index.js` remove the extra encoding and treat `pad.name` as already encoded when building the URL. Also consider displaying the decoded name to users (so they don’t see `%20` in the list).
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
(追記) View more (1) (追記ここまで)
4. Invalid timestamp crashes lobby 🐞 Bug ☼ Reliability
Description
margin/index.js filters recentPads entries only by name and then unconditionally formats `new Date(pad.timestamp) via toLocaleDateString(), which throws if timestamp` is missing or invalid. A single malformed entry can abort recent-pad rendering and break the lobby’s recent pads UI.
Evidence
The parsing step only checks for a name string, but later code sorts by timestamp and calls toLocaleDateString() on a Date constructed from pad.timestamp without validating that it is present/valid.
src/static/skins/margin/index.js[55-69]
src/static/skins/margin/index.js[71-75]
src/static/skins/margin/index.js[134-147]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ## Issue description `recentPads` entries are only validated for `name`, but `timestamp` is used for sorting and formatting without validation. Invalid timestamps can throw when calling `toLocaleDateString()`. ### Issue Context The code explicitly tries to tolerate corrupted `localStorage` entries, but it does not validate `timestamp`. ### Fix Focus Areas - src/static/skins/margin/index.js[60-75] - src/static/skins/margin/index.js[134-147] ### What to change - When parsing `recentPads`, additionally validate `timestamp` (and optionally `members`). - Example: keep only entries where `typeof p.timestamp === 'string'` and `!Number.isNaN(Date.parse(p.timestamp))`. - In rendering, guard formatting: - If `Date.parse()` is NaN, either skip the date, show a placeholder, or fall back to `''`. - In the sort comparator, handle invalid dates deterministically (e.g., treat invalid as oldest).
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
5. Lobby theme not validated 🐞 Bug ☼ Reliability ⭐ New
Description
margin/index.js applies marginTheme from localStorage directly to <html data-theme> without validating against supported themes, so a malformed value can leave CSS variables undefined and break lobby styling.
Evidence
index.js reads and applies the stored theme without checking it, while pad.js contains explicit validation logic for the same theme values—demonstrating the intended behavior that the lobby currently lacks.
src/static/skins/margin/index.js[14-18]
src/static/skins/margin/pad.js[34-37]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ## Issue description The lobby bootstrap (`margin/index.js`) sets `data-theme`/`data-mode` from localStorage without validating `theme` against the supported set, unlike `pad.js` which explicitly validates and falls back. ## Issue Context If `marginTheme` is corrupted or manually set to an unknown value, `[data-theme]` rules will apply but theme variables (e.g., `--m-bg`) won’t be defined by any of the theme blocks. ## Fix Focus Areas - src/static/skins/margin/index.js[7-22] - src/static/skins/margin/pad.js[34-44] ## Implementation notes - Add an allowlist check in `index.js` (mirror `pad.js`’s `MARGIN_THEME_VALUES.includes(theme)` fallback). - After normalizing the theme, compute the default mode from `MARGIN_MODE_DEFAULTS[theme]` when the stored mode is missing/invalid.
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
6. Missing final newline 🐞 Bug ⚙ Maintainability ⭐ New
Description
Several newly added margin skin files are missing a trailing newline despite .editorconfig requiring insert_final_newline = true, which creates noisy diffs and can fail editorconfig-based checks.
Evidence
The repository mandates final newlines via .editorconfig, and the added import-export.css (and several others in the PR diff) are flagged as missing a final newline.
.editorconfig[3-9]
src/static/skins/margin/src/components/import-export.css[1-5]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ## Issue description Multiple added skin assets are missing the final newline required by the repository’s EditorConfig settings. ## Issue Context This is a formatting consistency requirement (`insert_final_newline = true`). The PR diff explicitly flags missing newlines on multiple newly added files. ## Fix Focus Areas - .editorconfig[3-9] - src/static/skins/margin/src/components/import-export.css[1-5] ## Implementation notes - Ensure every newly added `.css`/`.js`/`.md` file ends with a single trailing newline. - Consider running an EditorConfig-aware formatter across `src/static/skins/margin/**`.
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1
to
+3
# margin G�� Etherpad skin
A standalone drop-in skin with six themes and an orthogonal Light/Dark toggle:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. Non-utf8 skin files 🐞 Bug ☼ Reliability
Several newly added margin skin assets contain invalid UTF-8 byte sequences (visible as mojibake like "G��" / "Sekund+�r"), which can break tooling that reads these files as UTF-8 and risks serving garbled text to browsers. This needs re-encoding/cleanup before merge to avoid brittle builds and hard-to-debug asset issues.
Agent Prompt
### Issue description
Multiple newly added `margin` skin files contain invalid UTF-8 bytes (mojibake sequences like `G��` / `+�`), which breaks UTF-8 decoding in tooling and can result in corrupted comments/text in served assets.
### Issue Context
These files are static assets (CSS/JS/MD) that are expected to be UTF-8 in typical web/build pipelines.
### Fix Focus Areas
- src/static/skins/margin/README.md[1-61]
- src/static/skins/margin/index.css[1-43]
- src/static/skins/margin/pad.css[1-416]
- src/static/skins/margin/pad.js[1-253]
- src/static/skins/margin/timeslider.js[1-42]
- src/static/skins/margin/src/pad-editor.css[1-48]
- src/static/skins/margin/src/components/buttons.css[1-74]
### What to change
- Convert the listed files to valid UTF-8.
- Replace any corrupted punctuation/characters in comments (or remove them) so the repository contains clean UTF-8 sources.
- Re-run whatever formatter/linter/build step is used for static assets to ensure no further encoding regressions.
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+104
to
+113
// Use new URL() so a trailing slash, query string, or hash on
// window.location.href doesn't produce a broken link, and so pad
// names with characters that need encoding still resolve.
const padPath = new URL(`p/${encodeURIComponent(pad.name)}`,
window.location.href).href;
const link = document.createElement('a');
link.style.textDecoration = 'none';
link.href = padPath;
link.innerText = pad.name;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. Recent pad link broken 🐞 Bug ≡ Correctness
margin/pad.js stores the pad name directly from window.location.pathname (percent-encoded), but margin/index.js applies encodeURIComponent() again when building the recent-pad URL, which double-encodes and breaks navigation for pad names containing spaces or non-ASCII characters. Users will click a recent pad and be sent to a different (wrong) pad URL (e.g., %20 becomes %2520).
Agent Prompt
### Issue description
Recent pad links can be double-encoded: the stored pad name is derived from `window.location.pathname` (already percent-encoded) and later encoded again with `encodeURIComponent()` when generating the link.
### Issue Context
Core navigation encodes pad names when generating `/p/<pad>` URLs, so the pathname segment can contain `%..` sequences.
### Fix Focus Areas
- src/static/skins/margin/pad.js[218-249]
- src/static/skins/margin/index.js[104-113]
### What to change
Choose ONE consistent representation for `recentPads[].name`:
1) **Store decoded names**: in `pad.js`, set `padName = decodeURIComponent(lastSegment)` before saving. Keep `encodeURIComponent(pad.name)` in `index.js`.
OR
2) **Store encoded names**: keep current storage, but in `index.js` remove the extra encoding and treat `pad.name` as already encoded when building the URL.
Also consider displaying the decoded name to users (so they don’t see `%20` in the list).
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+60
to
+147
try {
const recentPadsFromLocalStorage = localStorage.getItem('recentPads');
if (recentPadsFromLocalStorage != null) {
const parsed = JSON.parse(recentPadsFromLocalStorage);
if (Array.isArray(parsed)) {
recentPadListData = parsed.filter(
(p) => p && typeof p === 'object' && typeof p.name === 'string');
}
}
} catch (_) { /* private mode / corrupted entry */ }
// Remove duplicates based on pad name and sort by timestamp
recentPadListData = recentPadListData.filter(
(pad, index, self) => index === self.findIndex((p) => p.name === pad.name)
).sort((a, b) => new Date(a.timestamp) > new Date(b.timestamp) ? -1 : 1);
if (recentPadList && recentPadListData.length === 0) {
const parentStyle = recentPadList.parentElement.style;
recentPadListHeading.setAttribute('data-l10n-id', 'index.recentPadsEmpty');
parentStyle.display = 'flex';
parentStyle.justifyContent = 'center';
parentStyle.alignItems = 'center';
parentStyle.maxHeight = '100%';
recentPadList.remove();
} else if (recentPadList) {
/**
* @typedef {Object} Pad
* @property {string} name
*/
/**
* @param {Pad} pad
*/
const arrowIcon = '<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="lucide lucide-arrow-right w-4 h-4 text-gray-400"><path d="M5 12h14"></path><path d="m12 5 7 7-7 7"></path></svg>';
const clockIcon = '<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="lucide lucide-clock w-3 h-3"><circle cx="12" cy="12" r="10"></circle><polyline points="12 6 12 12 16 14"></polyline></svg>';
const personalIcon = '<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="lucide lucide-users w-3 h-3"><path d="M16 21v-2a4 4 0 0 0-4-4H6a4 4 0 0 0-4 4v2"></path><circle cx="9" cy="7" r="4"></circle><path d="M22 21v-2a4 4 0 0 0-3-3.87"></path><path d="M16 3.13a4 4 0 0 1 0 7.75"></path></svg>';
recentPadListData.forEach((pad) => {
const li = document.createElement('li');
li.style.cursor = 'pointer';
li.className = 'recent-pad';
// Use new URL() so a trailing slash, query string, or hash on
// window.location.href doesn't produce a broken link, and so pad
// names with characters that need encoding still resolve.
const padPath = new URL(`p/${encodeURIComponent(pad.name)}`,
window.location.href).href;
const link = document.createElement('a');
link.style.textDecoration = 'none';
link.href = padPath;
link.innerText = pad.name;
li.appendChild(link);
const arrowIconElement = document.createElement('span');
arrowIconElement.className = 'recent-pad-arrow';
arrowIconElement.innerHTML = arrowIcon;
li.appendChild(arrowIconElement);
const nextRow = document.createElement('div');
nextRow.style.display = 'flex';
nextRow.style.gap = '10px';
nextRow.style.marginTop = '10px';
const clockIconElement = document.createElement('span');
clockIconElement.className = 'recent-pad-clock';
clockIconElement.innerHTML = clockIcon;
nextRow.appendChild(clockIconElement);
const time = new Date(pad.timestamp);
const userLocale = navigator.language || 'en-US';
const formattedTime = time.toLocaleDateString(userLocale, {
year: 'numeric',
month: '2-digit',
day: '2-digit',
hour: '2-digit',
minute: '2-digit',
});
const timeElement = document.createElement('span');
timeElement.className = 'recent-pad-time';
timeElement.innerText = formattedTime;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3. Invalid timestamp crashes lobby 🐞 Bug ☼ Reliability
margin/index.js filters recentPads entries only by name and then unconditionally formats `new Date(pad.timestamp) via toLocaleDateString(), which throws if timestamp` is missing or invalid. A single malformed entry can abort recent-pad rendering and break the lobby’s recent pads UI.
Agent Prompt
### Issue description
`recentPads` entries are only validated for `name`, but `timestamp` is used for sorting and formatting without validation. Invalid timestamps can throw when calling `toLocaleDateString()`.
### Issue Context
The code explicitly tries to tolerate corrupted `localStorage` entries, but it does not validate `timestamp`.
### Fix Focus Areas
- src/static/skins/margin/index.js[60-75]
- src/static/skins/margin/index.js[134-147]
### What to change
- When parsing `recentPads`, additionally validate `timestamp` (and optionally `members`).
- Example: keep only entries where `typeof p.timestamp === 'string'` and `!Number.isNaN(Date.parse(p.timestamp))`.
- In rendering, guard formatting:
- If `Date.parse()` is NaN, either skip the date, show a placeholder, or fall back to `''`.
- In the sort comparator, handle invalid dates deterministically (e.g., treat invalid as oldest).
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+104
to
+108
// Use new URL() so a trailing slash, query string, or hash on
// window.location.href doesn't produce a broken link, and so pad
// names with characters that need encoding still resolve.
const padPath = new URL(`p/${encodeURIComponent(pad.name)}`,
window.location.href).href;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. Recent pad links break 🐞 Bug ≡ Correctness
margin/pad.js stores the pad name directly from window.location.pathname (already percent-encoded), but margin/index.js builds recent-pad URLs using encodeURIComponent(pad.name), causing double-encoding (e.g., my%20pad → my%2520pad) and broken navigation from the lobby’s recent list.
Agent Prompt
## Issue description
Recent pad links in the lobby can become invalid because pad names are persisted in `recentPads` in an already-encoded form (from `location.pathname`), then encoded again in `index.js` when constructing the URL.
## Issue Context
- `pad.js` stores the last path segment from `location.pathname` as the pad name.
- Browsers keep `location.pathname` percent-encoded.
- `index.js` encodes `pad.name` again via `encodeURIComponent()`.
## Fix Focus Areas
- src/static/skins/margin/pad.js[218-252]
- src/static/skins/margin/index.js[104-114]
## Implementation notes
- When capturing the pad name in `pad.js`, safely decode it before storing (wrap `decodeURIComponent()` in try/catch).
- When rendering the recent list in `index.js`, treat stored names as decoded and apply `encodeURIComponent()` exactly once when building the URL.
- Consider also displaying the decoded name in the UI (so users don’t see `%20`).
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Port of #7721
Rebased onto latest develop for mergeability.