add toolbar customization over drag & drop#8864
add toolbar customization over drag & drop#8864tuuuni-cell wants to merge 17 commits intoTriliumNext:mainfrom
Conversation
Users can now reorder, hide and group toolbar buttons directly from the text note settings.
…ble pool chips - Layout: active items now shown as a full-width vertical list (top = leftmost in editor) - Available items shown as compact chips below — click to append OR drag to insert - Removed 'Block Toolbar' tab (internal CKEditor detail, not user-facing) - Group rows now have a blue left border and bold label for clear visual hierarchy - Expanded group children shown with blue left indent line - Improved translation strings with clearer descriptions - Pool chips: click-to-add support in addition to drag-and-drop
…l chip hover - Replace braille ⠿ character with a proper inline SVG 2×3 dot grid (no font rendering issues) - Add border-bottom on every row for clear visual separation between items - Group rows: blue left border (4px) + primary-bg-subtle background + bold label - Expanded group children: shown on a distinct secondary-bg background - Pool chips: white background + border, blue hover (matches group color) - Drop indicator line: 3px rounded blue bar (was 2px, harder to see) - Extract design token constants (COLOR) for consistent theming
- Dark mode text: add explicit color:var(--bs-body-color) to all rows so text is always readable regardless of theme background - Group row bg: replace --bs-primary-bg-subtle (broken in dark mode) with rgba(13,110,253,0.13) — adds a blue tint to whatever body-bg is, works in both themes - Pool chip hover: replace background change with box-shadow ring — no background change means text stays readable in dark mode - DropLine: height is now 0 when inactive (no invisible 3px gaps between rows); expands to 4px bright blue when dragging between two specific rows - Drag active→pool to remove: pool section now has dragOver/drop handlers; when dragging an active item a dashed border + "drop to remove" hint appears; on hover turns red so the action is obvious
…e, better separator/dropline - Remove className="btn btn-link btn-sm p-0" from RemoveBtn and GroupRow toggle: these were being styled by Trilium's theme as wide gray rectangles, making rows look cluttered. Now use purely inline styles (background:none, border:none). - Remove className="btn btn-sm" from PoolChip for the same reason. - Active list now scrollable: maxHeight 380px + overflowY auto (was overflow:hidden). - Sticky delete zone: a "🗑 Drag here to remove" bar sticks to the bottom of the scrollable list area while dragging, turning red on hover. Always reachable regardless of list scroll position. - Separator row: replace near-invisible dashed border-top with solid 1px lines + a small badge label — clearly visible in both light and dark mode. - DropLine: replaced with VS Code-style indicator — 2px blue horizontal line with end-cap circles at both sides. Only rendered when active (no height when inactive). - Group bg opacity increased from 0.13 to 0.18 for more visible distinction.
…header contrast - Separator row: remove badge (background was resolving to wrong color in some themes); now uses "currentColor" lines at opacity 0.55 — automatically adapts to body text color in any theme, no background variables involved - Drop indicator (DropLine): replace clipped position:absolute circles with a solid 3px blue line + box-shadow glow; glow makes it unmissable in both light and dark mode - Row hover: replace background change (var(--bs-tertiary-bg) was inconsistent) with "inset 0 0 0 999px rgba(255,255,255,0.06)" box-shadow overlay — in dark mode adds a subtle lightness, reliable in any theme - Section headers: use var(--bs-body-color) at opacity 0.65 instead of var(--bs-secondary-color) which was too faint in some dark themes - rowSep: use solid border-color instead of translucent variant (more visible in dark mode)
The container's onDragOver was firing after every row's onDragOver due to event bubbling and always overwriting activeDrop with entries.length, so the blue line appeared only at the very bottom instead of between the two items the cursor was positioned between. Fix: add e.stopPropagation() in onActiveRowOver so the container handler only fires when the cursor is in empty space below all rows (which is the one remaining valid case for the container-level fallback). onChildRowOver already had stopPropagation — only the top-level row handler was missing it.
…ar-TzrEM Claude/customize trilium toolbar tzr em
dosubot
Bot
added
the
size:XXL
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a highly anticipated feature: interactive customization of the text editor's toolbar. Users can now personalize their editing environment by arranging, adding, and removing toolbar buttons and groups through an intuitive drag-and-drop interface. This enhancement significantly improves user control over the editor's layout, adapting it to individual workflows and preferences, and ensures a more efficient and tailored writing experience. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a powerful and well-designed feature for customizing the editor toolbar. The implementation is comprehensive, covering drag-and-drop reordering, adding/removing items, and grouping. The code is split into a configuration model, a UI component for customization, and integration into the existing toolbar logic, which is a good separation of concerns. I've identified a few areas for improvement, mainly concerning type safety, performance, and internationalization in the new UI component. Overall, this is a great addition.
| if (typeof item === "object" && "items" in item) { | ||
| for (const subitem of item.items) { | ||
| for (const subitem of (item as { items: string[] }).items) { | ||
| items.push(subitem); | ||
| } | ||
| } else { | ||
| items.push(item); | ||
| items.push(item as string); | ||
| } |
There was a problem hiding this comment.
The type assertion (item as { items: string[] }) is incorrect. The items property of a toolbar group object is of type (string | object)[], not string[]. This could lead to runtime errors if a sub-item is an object and is pushed into the items array, which is of type string[]. The loop should be more type-safe.
if (typeof item === "object" && "items" in item) {
for (const subitem of (item as { items: (string|object)[] }).items) {
if (typeof subitem === 'string') {
items.push(subitem);
}
}
} else if (typeof item === 'string') {
items.push(item);
}| const [local, setLocal] = useState<ToolbarCustomConfig>(() => parseConfig(savedRaw)); | ||
| const [tab, setTab] = useState<TabKey>("classic"); | ||
|
|
||
| const isDirty = JSON.stringify(local) !== JSON.stringify(parseConfig(savedRaw)); |
There was a problem hiding this comment.
For performance optimization, parseConfig(savedRaw) is called on every render. This can be memoized using useMemo to avoid re-parsing the JSON string unnecessarily.
const savedConfig = useMemo(() => parseConfig(savedRaw), [savedRaw]);
const isDirty = JSON.stringify(local) !== JSON.stringify(savedConfig);| function addSeparator() { commit([...entries, { kind: "separator" } as ToolbarSeparator]); } | ||
|
|
||
| function addGroup() { | ||
| const id = `group_${Date.now()}`; |
There was a problem hiding this comment.
Date.now() does not guarantee unique IDs, especially if a user adds items very quickly. This could lead to key conflicts in React. Using crypto.randomUUID() is a more robust way to generate unique IDs.
| const id = `group_${Date.now()}`; | |
| const id = `group_${crypto.randomUUID()}`; |
| clearDrag(); | ||
| }} | ||
| > | ||
| {poolOver ? "⬇ Release to remove from toolbar" : "🗑 Drag here to remove from toolbar"} |
There was a problem hiding this comment.
The strings "⬇ Release to remove from toolbar" and "🗑 Drag here to remove from toolbar" are hardcoded. They should be internationalized using the t() function. Similarly, on line 566, "⬇ Release to remove" is also hardcoded.
Please add these strings to apps/client/src/translations/en/translation.json and use t() to render them.
…strings - Replace Date.now() with crypto.randomUUID() to guarantee unique group IDs even when adding items rapidly - Add i18n keys for the three hardcoded drag/drop removal strings and use t() calls instead of raw English text https://claude.ai/code/session_017f29o9ho9X2q69LiyLj1nj
fix: use crypto.randomUUID() for group IDs and internationalize drag …
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature for interactive toolbar customization with drag and drop functionality. While a great addition, it introduces a stored Cross-Site Scripting (XSS) vulnerability. The custom toolbar configuration, stored in a user-controlled option, contains properties that are rendered unsanitized as HTML/SVG by the underlying editor component, potentially allowing arbitrary JavaScript execution if an attacker manipulates the configuration. It is crucial to add sanitization or strict validation for the toolbar group labels and icons to mitigate this risk. Additionally, the code review suggests improvements for the new ToolbarCustomization component, including using CSS classes instead of inline styles, enhancing usability by allowing items to be dragged out of groups, and following React best practices for conditional rendering.
| label: entry.label, | ||
| icon: resolveIcon(entry.icon), |
There was a problem hiding this comment.
The textNoteToolbarConfig option stores a JSON-serialized configuration for the text note toolbar. This configuration includes ToolbarGroup objects, which have label and icon properties. These properties are passed unsanitized to CKEditor 5 when building the toolbar. CKEditor 5 renders the icon property as an SVG string, which can contain malicious JavaScript (e.g., <svg/onload=alert(1)>). Since the textNoteToolbarConfig option is user-controlled and added to the ALLOWED_OPTIONS list in the server-side API, an authenticated user (or an attacker via CSRF) can set a malicious configuration that will execute JavaScript whenever the user opens a text note.
To remediate this, sanitize the label and icon properties of ToolbarGroup objects before passing them to CKEditor. Alternatively, restrict the icon property to a set of allowed built-in icon names or validate that it is a safe SVG string.
| function onActiveDrop(e: DragEvent, at: number) { | ||
| e.preventDefault(); | ||
| if (!drag) return; | ||
| if (drag.from === "pool") insertAt(drag.id, at); | ||
| else if (drag.from === "active") moveActive(drag.idx, at); | ||
| clearDrag(); | ||
| } |
There was a problem hiding this comment.
The current implementation doesn't allow dragging an item from a group to the main list of active items. This can be inconvenient for users who want to reorganize their toolbar by moving items out of groups. Consider enhancing this drop handler to support items dragged from a group (drag.from === 'child'). A similar enhancement could be made to onGroupRowDrop to allow moving items between groups.
| function ItemRow({ id, faded, indent, onDragStart, onDragEnd, onDragOver, onDrop, onRemove }: RowBase & { id: string }) { | ||
| return ( | ||
| <div | ||
| draggable | ||
| style={rowBase(faded, { paddingLeft: indent ? "28px" : "8px" })} | ||
| onDragStart={onDragStart} onDragEnd={onDragEnd} | ||
| onDragOver={onDragOver} onDrop={onDrop} | ||
| onMouseEnter={e => (e.currentTarget as HTMLElement).style.boxShadow = `inset 0 0 0 999px ${COLOR.hoverOverlay}`} | ||
| onMouseLeave={e => (e.currentTarget as HTMLElement).style.boxShadow = ""} | ||
| > | ||
| <DragDots /> | ||
| <span style={{ display: "inline-flex", alignItems: "center", justifyContent: "center", width: "18px", flexShrink: 0 }}> | ||
| <ToolbarIcon id={id} size={14} /> | ||
| </span> | ||
| <span style={{ marginLeft: "8px", flex: 1, fontSize: "0.86em", overflow: "hidden", textOverflow: "ellipsis", whiteSpace: "nowrap" }}> | ||
| {getItemLabel(id)} | ||
| </span> | ||
| <RemoveBtn onClick={onRemove} /> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
This component, along with SepRow, GroupRow, and PoolChip, uses inline styles and direct DOM manipulation for hover effects (onMouseEnter, onMouseLeave). This is generally considered an anti-pattern in React as it mixes presentation with logic and can be less performant. It would be better to use CSS classes and the :hover pseudo-selector for these effects. This would improve maintainability and readability.
For example:
.item-row:hover {
box-shadow: inset 0 0 0 999px rgba(255,255,255,0.06);
}Additionally, the event handler types in the RowBase interface could be more specific (e.g., onDrop: (e: DragEvent) => void;) to avoid repeated type casting (e as DragEvent) in the component implementations.
| * When active: a solid 3px blue line with a bright glow so it's unmissable in any theme. | ||
| */ | ||
| function DropLine({ active, indent }: { active: boolean; indent?: boolean }) { | ||
| if (!active) return <div style={{ height: 0 }} />; |
There was a problem hiding this comment.
Instead of rendering a div with height: 0, it's more idiomatic in React to return null when a component should not render anything. This avoids adding an unnecessary element to the DOM.
| if (!active) return <div style={{ height: 0 }} />; | |
| if (!active) return null; |
tuuuni-cell
changed the title
3 participants
Adds a new "Toolbar" tab to the Text Notes options panel, allowing users to
fully customize the toolbar.
Its my first time contributing do you need something more?