fix: restore table controls and improve popup positioning #480 #485
Conversation
WalkthroughThe tiptap bubble menu component was restructured: an outer wrapper now contains two BubbleMenu instances—the original controlled by 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @NagariaHussain, I’m following up on this PR. It restores the missing table manipulation controls and fixes the menu positioning as described in issue #480. All automated checks and CodeRabbit validations have passed. Could you please take a look when you have a moment? Thanks! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/tiptap-extensions/WikiBubbleMenu.vue (1)
199-209:⚠️ Potential issue | 🟠 MajorBoth bubble menus will appear simultaneously when text is selected inside a table.
shouldShowBubbleMenudoesn't exclude the table context, so selecting text within a table cell will trigger both the formatting menu and the table menu at the same time, likely overlapping each other.Add a table check to
shouldShowBubbleMenu:Proposed fix
function shouldShowBubbleMenu({ editor, state }) { const selection = state.selection; // Bubble menu is only for inline text formatting, not media/node selections. if (selection instanceof NodeSelection) return false; if (selection.empty) return false; if (editor.isActive('videoBlock')) return false; if (editor.isActive('image')) return false; + if (editor.isActive('table')) return false; return true; }Alternatively, if formatting inside tables should still be possible, consider a toggle or a combined menu approach so both don't render independently.
🧹 Nitpick comments (3)
frontend/src/components/tiptap-extensions/WikiBubbleMenu.vue (3)
131-135: Table menu'stippy-optionsis missingpreventOverflowandflipmodifiers.The primary bubble menu (line 7–28) includes
preventOverflowandflippopper modifiers to keep it within the viewport. The table menu omits these, so it can be clipped or hidden when the table is near the edge of the viewport.Proposed fix
:tippy-options="{ duration: 100, zIndex: 50, offset: [0, 10] }" + <!-- Consider matching the primary menu's popperOptions: + popperOptions: { + modifiers: [ + { name: 'preventOverflow', options: { boundary: 'viewport', padding: 8 } }, + { name: 'flip', options: { fallbackPlacements: ['top', 'bottom', 'right'] } } + ] + } + -->A cleaner approach — extract the shared tippy config into a computed/const and reuse it:
const sharedTippyOptions = { duration: 100, zIndex: 50, popperOptions: { modifiers: [ { name: 'preventOverflow', options: { boundary: 'viewport', padding: 8 } }, { name: 'flip', options: { fallbackPlacements: ['top', 'bottom', 'right'] } }, ], }, };Then for the table menu, spread and add the offset:
{ ...sharedTippyOptions, offset: [0, 10] }.
139-165: Hardcoded hex colors bypass the design system.The
Trash2icons use inlinecolor="#ef4444"andcolor="#b91c1c", while every other color in this component uses CSS custom properties (e.g.,var(--ink-gray-6,#4b5563)). This will not respond to theme changes and is inconsistent.Consider using a CSS class instead:
Proposed fix
- <Trash2 :size="16" color="#ef4444" /> + <Trash2 :size="16" class="text-red-500" />- <Trash2 :size="16" color="#b91c1c" /> + <Trash2 :size="16" class="text-red-700" />Or define a CSS custom property for destructive actions and use it here.
139-143: Replace icon rotation with purpose-built table operation icons.The current icons fail to provide clear visual differentiation:
PlusSquareis symmetric (180° rotation is visually identical), andColumnsrotated 90° is unconventional. lucide-vue-next provides dedicated icons specifically designed for table operations:
- Replace rotated
PlusSquareicons withTableRowsSplitfor row insert operations- Replace rotated
Columnsicons withTableColumnsSplitfor column insert operationsThis improves icon clarity and makes actions more discoverable without relying on rotation or unconventional transformations.
…, and fixing responsive layout
Valley-15
force-pushed
the
fix/table-row-deletion-480
branch
from
76b329a to
6ff741a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wiki/templates/wiki/layout.html`:
- Around line 55-60: Fix the inconsistent indentation inside the template by
aligning the inner <div> and the closing </main> with the opening <main> tag and
surrounding context; update the block body lines inside the <main> (the {% block
body %}{% endblock %}) so they use the same indentation level as <main>’s
children (i.e., match the existing 8-space indentation), ensuring <main
class="...">, its child <div>, the block body, and the closing </main> are
consistently indented.
🧹 Nitpick comments (2)
frontend/src/components/tiptap-extensions/WikiBubbleMenu.vue (2)
139-165: Three identical trash icons may confuse users.The delete row (Line 146), delete column (Line 158), and delete table (Line 164) buttons all use
Trash2with only a slight color difference (text-red-500vstext-red-700). The visual distinction is minimal. Consider using more descriptive icons or adding text labels to improve discoverability. The "Add Row Above" icon (rotatedTableRowsSplit) may also not be immediately intuitive.
131-167: Table BubbleMenu — consider addingpreventOverflowlike the main menu.The main BubbleMenu (Line 7-28) has
preventOverflowandflippopper modifiers to keep it within the viewport, but the table BubbleMenu (Line 135) only setsoffset: [0, 10]. For tables near viewport edges, the table menu could get clipped.♻️ Suggested tippy-options for the table menu
- :tippy-options="{ duration: 100, zIndex: 50, offset: [0, 10] }" + :tippy-options="{ + duration: 100, + zIndex: 50, + offset: [0, 10], + popperOptions: { + modifiers: [ + { + name: 'preventOverflow', + options: { + boundary: 'viewport', + padding: 8, + }, + }, + { + name: 'flip', + options: { + fallbackPlacements: ['top', 'bottom'], + }, + }, + ], + }, + }"
Valley-15
force-pushed
the
fix/table-row-deletion-480
branch
from
871fb1d to
25ba25a
Compare
|
@NagariaHussain, I’ve addressed all the feedback. You can see everything working perfectly in the attached recording. Updates: Fixed Menu Overlap: Added a robust crawler to suppress the formatting menu inside tables. Icon Polish: Switched to cohesive TableRowsSplit and TableColumnsSplit icons (with mirroring for columns). Design System: Swapped hardcoded hex colors for standard CSS classes. Stability: Added preventOverflow and flip to the table menu. Layout: Cleaned up indentation and structure in layout.html. Conflicts are resolved and the branch is synced with develop 0a7.mp4 |
1 participant
Problem
Table management controls (adding/deleting rows and columns) were missing from the UI editor.
Changes
BubbleMenufor tables inWikiBubbleMenu.vue.10px vertical offsetto the menu positioning to ensure it does not overlap cell content.Preview
Fixes #480
Summary by CodeRabbit
New Features
Style