Style: Larger Node Text, More Sidebar Alignment #7223
Merged
Conversation
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
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (14)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughWalkthroughSearchBox was refactored and moved from Changes
Possibly related PRs
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/12/2025, 01:17:29 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results❌ Some tests failed ⏰ Completed at: 12/12/2025, 01:27:13 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
DrJKL
force-pushed
the
drjkl/reverse-shrink-ray
branch
from
6479bac to
30f9668
Compare
DrJKL
force-pushed
the
drjkl/consistency-is-inter
branch
2 times, most recently
from
d2ebe0f to
b702361
Compare
DrJKL
force-pushed
the
drjkl/reverse-shrink-ray
branch
from
25bf976 to
b0476bd
Compare
DrJKL
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
github-actions
bot
removed
the
New Browser Test Expectations
DrJKL
force-pushed
the
drjkl/consistency-is-inter
branch
3 times, most recently
from
b1f64a6 to
b7d26e9
Compare
DrJKL
force-pushed
the
drjkl/reverse-shrink-ray
branch
from
8197219 to
bc7d097
Compare
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.23 MB (baseline 3.23 MB) • 🟢 -1.54 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 981 kB (baseline 981 kB) • 🟢 -7 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • 🟢 -2 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 178 kB (baseline 178 kB) • 🟢 -323 BReusable component library chunks
Status: 9 added / 9 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 3 added / 3 removed Utilities & Hooks — 3.18 kB (baseline 3.18 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • 🔴 +418 BExternal libraries and shared vendor chunks
Status: 5 added / 5 removed Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
Status: 23 added / 23 removed |
DrJKL
changed the title
DrJKL
added
Design
DrJKL
force-pushed
the
drjkl/reverse-shrink-ray
branch
from
ae7fa98 to
e258e03
Compare
DrJKL
added
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
DrJKL
force-pushed
the
drjkl/reverse-shrink-ray
branch
from
6745b42 to
71a25cc
Compare
DrJKL
force-pushed
the
drjkl/reverse-shrink-ray
branch
from
38b35be to
d4b028a
Compare
DrJKL
force-pushed
the
drjkl/reverse-shrink-ray
branch
from
b116250 to
f4e089b
Compare
![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: 1
♻️ Duplicate comments (10)
src/renderer/extensions/vueNodes/widgets/components/layout/WidgetLayoutField.vue (2)
21-25: Conditional rendering preserved from previous implementation.The
v-if="widget.name"condition matches the previous logic, as confirmed in past review discussions.
35-37: Event handlers simplified to modifier-only pattern.Using
.stopmodifiers without handler functions is cleaner and idiomatic Vue.src/components/common/SearchBox.stories.ts (1)
4-8: LGTM – Import path and typing workaround.The import path update and
GenericMetainterface work around typing issues with the exposedfocusmethod. This approach was previously reviewed and accepted.src/components/common/SearchBox.vue (2)
50-50: Use@/utils/tailwindUtilforcnimport.The coding guidelines specify using the local re-export from
@/utils/tailwindUtilrather than importing directly from@comfyorg/tailwind-utils.-import { cn } from '@comfyorg/tailwind-utils' +import { cn } from '@/utils/tailwindUtil'
107-119: Size prop is not applied whenshowBorderis true.The
wrapperStylecomputed property returns early whenshowBorderis true, bypassing the size-specific classes. Both props should work independently.const wrapperStyle = computed(() => { + const sizeClasses = { + md: 'h-8 px-2 py-1.5', + lg: 'h-10 px-4 py-2' + }[size] + if (showBorder) { - return cn('rounded p-2 border border-solid border-border-default') + return cn('rounded border border-solid border-border-default', sizeClasses) } - // Size-specific classes matching button sizes for consistency - const sizeClasses = { - md: 'h-8 px-2 py-1.5', // Matches button sm size - lg: 'h-10 px-4 py-2' // Matches button md size - }[size] - return cn('rounded-lg', sizeClasses) })src/renderer/extensions/vueNodes/components/NodeWidgets.vue (1)
54-63: Removingflex-1from a grid item is fine (grid sizing is viacol-span-*).src/components/sidebar/tabs/ModelLibrarySidebarTab.vue (1)
32-32: New PrimeVueDividerusage should be avoided / centralized (migration already tracked).
Please ensure this is aligned with the existing “replace Divider” effort and confirm correct PrimeVue import style for tree-shaking in this repo.Also applies to: 48-50
src/components/sidebar/tabs/AssetsSidebarTab.vue (1)
61-61: PrimeVueDividerintroduction: please align with the existing divider replacement plan.
Also confirm import style and bundle impact (import { Divider } from 'primevue'vs component import path used elsewhere).Also applies to: 179-183
src/renderer/extensions/vueNodes/components/InputSlot.vue (1)
3-18:socketlesshiding the entire slot still looks risky—confirm it’s only used withdot-only.
IfInputSlotis ever rendered with a label/field andsocketless=true, this would hide more than just the socket.#!/bin/bash # Verify InputSlot socketless usage contexts (dot-only vs full slot) rg -n --type=vue "<InputSlot\\b" -C2 echo "---- socketless call sites ----" rg -n --type=vue "socketless" -C3 echo "---- socketless + dot-only call sites (expected) ----" rg -n --type=vue "InputSlot[\\s\\S]{0,200}socketless[\\s\\S]{0,200}dot-only|dot-only[\\s\\S]{0,200}socketless" -nsrc/platform/assets/components/MediaAssetFilterBar.vue (1)
3-7: Avoid concatenating'...'outside the translation (i18n punctuation).
This is already tracked (see existing i18n ticket); ideally use a dedicated placeholder key that includes the ellipsis.
Comment on lines
+665
to
666
| "searchAssets": "Search Assets", | ||
| "labels": { |
Contributor
There was a problem hiding this comment.
Confirm this string is a label (not a placeholder) and stays consistent with other “Search…” copy.
sideToolbar.searchAssets changed to title case (“Search Assets”), but many other search strings are placeholder-style with ellipses (e.g., g.searchPlaceholder: “Search...”). If this is shown inside an input as placeholder text, the ellipsis/lowercase pattern may be preferable; if it’s a section label/header, title case is fine.
🤖 Prompt for AI Agents
In src/locales/en/main.json around lines 665-666, the string "searchAssets":
"Search Assets" must be validated as a label vs a placeholder and made
consistent with other "Search…" entries; search the codebase for
sideToolbar.searchAssets usage: if it's rendered as a placeholder inside an
input, change the value to the placeholder-style "Search..."
(lowercase/ellipsis) and align casing with g.searchPlaceholder; if it's used as
a section label or header, keep title case "Search Assets"; update the locale
string accordingly and ensure any tests/components expecting the previous format
are updated to match.
Contributor
Author
There was a problem hiding this comment.
What is the casing of the other search boxes, like nodes and models?
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the casing pattern of other search-related strings in the locales file
echo "=== Search-related strings in main.json ==="
cat src/locales/en/main.json | jq -r '.. | select(type == "string") | select(test("(?i)search"; "i"))' | grep -i search | head -30
echo ""
echo "=== Specific search strings from g and sideToolbar sections ==="
cat src/locales/en/main.json | jq -r '.g.searchWorkflows, .g.searchSettings, .g.searchNodes, .g.searchModels, .g.searchKeybindings, .g.searchExtensions, .g.search, .g.searchPlaceholder, .sideToolbar.searchAssets'
Length of output: 705
🏁 Script executed:
#!/bin/bash
# Check how SearchBox is used with these search strings in components
echo "=== Usage of searchAssets in MediaAssetFilterBar ==="
rg -A2 -B2 "searchAssets" src/platform/assets/components/MediaAssetFilterBar.vue
echo ""
echo "=== Usage of searchNodes in NodeLibrarySidebarTab ==="
rg -A2 -B2 'searchNodes|placeholder.*search' src/components/sidebar/tabs/NodeLibrarySidebarTab.vue
echo ""
echo "=== Usage of searchModels in ModelLibrarySidebarTab ==="
rg -A2 -B2 'searchModels|placeholder.*search' src/components/sidebar/tabs/ModelLibrarySidebarTab.vue
echo ""
echo "=== Usage in WorkflowsSidebarTab ==="
rg -A2 -B2 'searchWorkflows|placeholder.*search' src/components/sidebar/tabs/WorkflowsSidebarTab.vue
Length of output: 920
@DrJKL All the other search boxes use Title Case without ellipsis in the locale strings:
g.searchNodes: "Search Nodes"g.searchModels: "Search Models"g.searchWorkflows: "Search Workflows"g.searchSettings: "Search Settings"g.searchKeybindings: "Search Keybindings"
The components then add the ellipsis programmatically with + '...' when using these strings as placeholders:
:placeholder="$t('g.searchNodes') + '...'"
:placeholder="$t('g.searchModels') + '...'"
:placeholder="$t('sideToolbar.searchAssets') + '...'"So your change from "Search assets..." to "Search Assets" is perfectly consistent with the existing pattern! 👍
DrJKL
added
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
AustinMroz
approved these changes
Dec 12, 2025
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.
Summary
See what it looks like. How it feels. What do you think?
Changes
┆Issue is synchronized with this Notion page by Unito