Repository navigation
fix(files): keep Markdown tables readable while scrolling - #10041
waleedlatif1 wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 7 files
Confidence score: 3/5
test-markdown-viewer-e2e.tsis not wired into a package command or the HTTP E2E workflow, so its Markdown layout checks will not run in CI. Add a suite command and invoke it from the workflow.use-bubble-menu-floating.tscan leave the menu hidden after scrolling back because later scroll callbacks are ignored. Keep a recovery path so the menu can be repositioned and shown again.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/scripts/test-markdown-viewer-e2e.ts">
<violation number="1" location="apps/sim/scripts/test-markdown-viewer-e2e.ts:13">
P2: This new browser suite is not wired into a package command or the HTTP E2E workflow, so the Markdown layout regressions it covers will not run in CI. Add a suite command and invoke it from the E2E workflow with its loopback app, database, auth, realtime, and artifact configuration.</violation>
</file>
<file name="apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/menus/use-bubble-menu-floating.ts">
<violation number="1" location="apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/menus/use-bubble-menu-floating.ts:63">
P2: The visibility guard can permanently suppress recovery: after Tiptap hides the menu when the selection leaves a clipping ancestor, subsequent scroll callbacks are ignored, so scrolling back cannot reposition and show the toolbar. Dispatch the position update whenever the connected menu and editor are alive, regardless of its current visibility.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
| import postgres from 'postgres' | ||
|
|
||
| /** | ||
| * Browser layout and persistence proof against a disposable local app and database. |
There was a problem hiding this comment.
P2: This new browser suite is not wired into a package command or the HTTP E2E workflow, so the Markdown layout regressions it covers will not run in CI. Add a suite command and invoke it from the E2E workflow with its loopback app, database, auth, realtime, and artifact configuration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/sim/scripts/test-markdown-viewer-e2e.ts, line 13:
<comment>This new browser suite is not wired into a package command or the HTTP E2E workflow, so the Markdown layout regressions it covers will not run in CI. Add a suite command and invoke it from the E2E workflow with its loopback app, database, auth, realtime, and artifact configuration.</comment>
<file context>
@@ -0,0 +1,511 @@
+import postgres from 'postgres'
+
+/**
+ * Browser layout and persistence proof against a disposable local app and database.
+ * Set MARKDOWN_VIEWER_E2E_BASE_URL, MARKDOWN_VIEWER_E2E_DATABASE_URL,
+ * MARKDOWN_VIEWER_E2E_AUTH_SECRET, and MARKDOWN_VIEWER_E2E_REPORT_PATH.
</file context>
| const { node } = editor.view.domAtPos(editor.state.selection.from) | ||
| const reference = node instanceof Element ? node : (node.parentElement ?? editor.view.dom) | ||
| stop = autoUpdate(reference, menu, () => { | ||
| if (!editor.isDestroyed && menu.isConnected && menu.style.visibility !== 'hidden') { |
There was a problem hiding this comment.
P2: The visibility guard can permanently suppress recovery: after Tiptap hides the menu when the selection leaves a clipping ancestor, subsequent scroll callbacks are ignored, so scrolling back cannot reposition and show the toolbar. Dispatch the position update whenever the connected menu and editor are alive, regardless of its current visibility.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/menus/use-bubble-menu-floating.ts, line 63:
<comment>The visibility guard can permanently suppress recovery: after Tiptap hides the menu when the selection leaves a clipping ancestor, subsequent scroll callbacks are ignored, so scrolling back cannot reposition and show the toolbar. Dispatch the position update whenever the connected menu and editor are alive, regardless of its current visibility.</comment>
<file context>
@@ -35,21 +36,42 @@ function selectionVirtualElement(editor: Editor) {
+ const { node } = editor.view.domAtPos(editor.state.selection.from)
+ const reference = node instanceof Element ? node : (node.parentElement ?? editor.view.dom)
+ stop = autoUpdate(reference, menu, () => {
+ if (!editor.isDestroyed && menu.isConnected && menu.style.visibility !== 'hidden') {
+ editor.commands.setMeta(pluginKey, 'updatePosition')
+ }
</file context>
| if (!editor.isDestroyed && menu.isConnected && menu.style.visibility !== 'hidden') { | |
| if (!editor.isDestroyed && menu.isConnected) { |
 [Medium impact] The changes affect how people read and edit Markdown files across public and editable views. The PR appears safe to merge, with non-blocking follow-ups for collaborator names and toolbar test coverage. FindingsSummaryThis PR gives Markdown tables their own sideways scroll area and updates floating tools when their position or size changes.
Reviews (1) · Last reviewed commit: "fix(files): keep Markdown tables readabl..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| .rich-markdown-nodes .tableWrapper { | ||
| max-width: 100%; | ||
| overflow-x: auto; | ||
| } |
There was a problem hiding this comment.
A collaborator name can be clipped after the user scrolls a wide table. activateCaretLabel chooses the label’s direction only when the peer moves. A label that flipped left near the right edge keeps pointing left when scrolling moves its caret near the left edge, outside the new scroll box. Update the label’s direction on table scroll and keep it within the wrapper’s visible bounds.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| await last.click() | ||
| await proveToolbar(page, page.getByRole('toolbar', { name: 'Table editing' })) |
There was a problem hiding this comment.
The new browser suite never checks the two events that autoUpdate was added to handle. editLastCell scrolls the table before opening the toolbar, and proveToolbar checks only its current position. No check scrolls with the toolbar open or changes its controls, such as opening the link editor. Add those actions and assert the toolbar’s position afterward so removing the new observer makes the suite fail.
Summary
Type of Change
Testing
Checklist
test-auditauthoring gate)