fix: prevent confirm dialog buttons from being unreachable on mobile with long text#8746
Open
christian-byrne wants to merge 7 commits intomainfrom
Open
fix: prevent confirm dialog buttons from being unreachable on mobile with long text#8746christian-byrne wants to merge 7 commits intomainfrom
christian-byrne wants to merge 7 commits intomainfrom
Conversation
dosubot
bot
added
the
size:M
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/14/2026, 03:31:12 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
|
Playwright: ❌ 517 passed, 1 failed · 4 flaky ❌ Failed Tests📊 Browser Reports
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated confirmation dialog templates to improve handling of long/unbroken text (word-break and whitespace preservation), constrain dialog height with scrolling, and allow action buttons to wrap on narrow viewports; added unit and Playwright tests validating long-message rendering and responsive button layout. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
christian-byrne
commented
Feb 8, 2026
Contributor
Author
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
Add a playwright test
Comment on lines
43
to
47
| it('uses flex-wrap on button container for narrow viewports', () => { | ||
| const wrapper = mountComponent() | ||
| const buttonRow = wrapper.find('.flex.flex-wrap.justify-end.gap-4') | ||
| expect(buttonRow.exists()).toBe(true) | ||
| }) |
Contributor
Author
There was a problem hiding this comment.
Not necessary test
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 21.7 kB (baseline 21.7 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 879 kB (baseline 879 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.9 kB (baseline 68.9 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 427 kB (baseline 427 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16.1 kB (baseline 16.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 785 B (baseline 785 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 36.6 kB (baseline 36.6 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.15 MB (baseline 2.15 MB) • 🟢 -66 BStores, services, APIs, and repositories
Status: 12 added / 12 removed Utilities & Hooks — 237 kB (baseline 237 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 13 added / 13 removed Vendor & Third-Party — 8.69 MB (baseline 8.69 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.31 MB (baseline 7.31 MB) • ⚪ 0 BBundles that do not match a named category
Status: 51 added / 51 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/dialog/content/ConfirmationDialogContent.test.ts`:
- Around line 22-34: The mountComponent helper accepts an untyped props
parameter; change its signature to type props using ComponentProps so tests are
type-safe. Import ComponentProps from 'vue' and update mountComponent(props = {}
as Partial<ComponentProps<typeof ConfirmationDialogContent>>) (or similar
Partial<> wrapper) so the default spread and overrides remain valid; update any
related usages if necessary. This targets the mountComponent function and the
ConfirmationDialogContent component type.
In `@src/components/dialog/content/ConfirmationDialogContent.vue`:
- Around line 132-136: Remove the scoped <style> block that defines
.prompt-dialog-content and instead apply Tailwind classes on the section element
that uses that class; replace the CSS rules white-space: pre-wrap and
overflow-wrap: break-word by adding whitespace-pre-wrap and break-words to the
section's class list (replace or remove "prompt-dialog-content" usage in the
template and use the Tailwind utilities directly).
src/components/dialog/content/ConfirmationDialogContent.test.ts
Outdated
Show resolved
Hide resolved
…ops, add Playwright test Amp-Thread-ID: https://ampcode.com/threads/T-019c3f33-f3ea-726c-9b87-a5e07a6bcc62
…obile Add max-h-[70vh] to the dialog section and wrap message content in a scrollable container with overflow-y-auto, keeping the button row pinned with shrink-0. This ensures buttons remain in viewport even when long unbreakable text fills the dialog on small screens. Also applies formatter to the Playwright test file. Amp-Thread-ID: https://ampcode.com/threads/T-019c3f6e-3eb3-725c-bf54-0183361abe1d
dosubot
bot
added
size:L
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/dialog/content/ConfirmationDialogContent.vue`:
- Around line 2-6: The scroll container in ConfirmationDialogContent.vue
currently uses the div with class "flex-1 overflow-y-auto" which can fail to
shrink in flex columns; update that element (the div containing the message) to
include min-h-0 (e.g., change its class to include "min-h-0") so the
overflow-y-auto container can shrink reliably and allow scrolling without
pushing action buttons off-screen.
Comment on lines
2
to
6
| <section | ||
| class="m-2 mt-4 flex max-h-[70vh] flex-col gap-6 whitespace-pre-wrap break-words" | ||
| > | ||
| <div class="flex-1 overflow-y-auto"> | ||
| <span>{{ message }}</span> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/dialog/content/ConfirmationDialogContent.vueRepository: Comfy-Org/ComfyUI_frontend
Length of output: 4784
🏁 Script executed:
rg "overflow-y-auto" src/components --type vue -A 2 -B 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
rg "overflow-y-auto" src/components -A 2 -B 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 9356
Add min-h-0 to ensure the scroll container shrinks reliably in flex columns.
Without min-h-0, flex children with overflow-y-auto may not shrink below their content size in all browsers, preventing the scrollable area from functioning properly and allowing long content to push the action buttons off-screen. This pattern is consistently applied throughout the codebase (e.g., BaseModalLayout.vue, MembersPanelContent.vue).
✅ Suggested fix
- <div class="flex-1 overflow-y-auto">
+ <div class="min-h-0 flex-1 overflow-y-auto">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <section | |
| class="m-2 mt-4 flex max-h-[70vh] flex-col gap-6 whitespace-pre-wrap break-words" | |
| > | |
| <div class="flex-1 overflow-y-auto"> | |
| <span>{{ message }}</span> | |
| <section | |
| class="m-2 mt-4 flex max-h-[70vh] flex-col gap-6 whitespace-pre-wrap break-words" | |
| > | |
| <div class="min-h-0 flex-1 overflow-y-auto"> | |
| <span>{{ message }}</span> |
🤖 Prompt for AI Agents
In `@src/components/dialog/content/ConfirmationDialogContent.vue` around lines 2 -
6, The scroll container in ConfirmationDialogContent.vue currently uses the div
with class "flex-1 overflow-y-auto" which can fail to shrink in flex columns;
update that element (the div containing the message) to include min-h-0 (e.g.,
change its class to include "min-h-0") so the overflow-y-auto container can
shrink reliably and allow scrolling without pushing action buttons off-screen.
christian-byrne
force-pushed
the
fix/mobile/confirm-dialog-textbreak
branch
4 times, most recently
from
a3ad458 to
37d7095
Compare
christian-byrne
force-pushed
the
fix/mobile/confirm-dialog-textbreak
branch
from
ce93292 to
9b89c9c
Compare
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
2 participants
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.
Summary
Fix confirm dialog buttons becoming unreachable on mobile when text contains long unbreakable words (e.g. content-hashed filenames with 100+ characters).
Changes
overflow-wrap: break-wordandflex-wrapto both confirm dialog systems so long words break properly and buttons wrap on narrow screens.ConfirmationDialogContent.vue: Addedoverflow-wrap: break-wordto the existing scoped style andflex-wrapto button row.ConfirmBody.vue: Addedbreak-wordstailwind class.ConfirmFooter.vue: Addedflex-wrapto button section.Review Focus
Minimal CSS-only fix across both dialog systems (legacy
dialogService.confirm()and newershowConfirmDialog()). No behavioral changes.┆Issue is synchronized with this Notion page by Unito