-
Notifications
You must be signed in to change notification settings - Fork 460
Component: Button Migration 3: IconTextButton #7603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughDeleted the IconTextButton component and its Storybook stories, removed the buttonTypes.ts types/helpers module, and replaced IconTextButton usages across many components with the unified Button component, adjusting props, inline icon/label markup, some size typings, and a few i18n call sites. Changes
Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/18/2025, 11:03:11 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results❌ Some tests failed ⏰ Completed at: 12/18/2025, 11:12:04 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.2 MB (baseline 3.21 MB) • 🟢 -2.78 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 996 kB (baseline 996 kB) • 🟢 -620 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 — 297 kB (baseline 297 kB) • 🟢 -2 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 184 kB (baseline 184 kB) • 🔴 +83 BReusable component library chunks
Status: 8 added / 8 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 1.86 kB (baseline 1.86 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.46 MB (baseline 8.46 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.44 MB (baseline 3.44 MB) • ⚪ 0 BBundles that do not match a named category
Status: 20 added / 20 removed |
908427e to
ba19ff4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
src/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (3)
src/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue (1)
12-12: Unconditional spinner renders always - add v-if condition.The DotSpinner is rendered unconditionally, causing it to spin even when the button is not performing an update. This creates a confusing UX where the loading indicator is always visible.
🔎 Proposed fix
- <DotSpinner duration="1s" :size="12" /> + <DotSpinner v-if="isUpdating" duration="1s" :size="12" />src/platform/assets/components/MediaAssetSortMenu.vue (1)
3-10: Minor inconsistency in text rendering (duplicate).Line 8 wraps the translated text in
<span>, while lines 17, 27, and 36 render text directly. This inconsistency was already flagged in a previous review.src/components/widget/layout/BaseModalLayout.stories.ts (1)
210-228: Fix deprecated Button usage to match the pattern used elsewhere in this file.These Buttons still use the deprecated
labelprop and#icontemplate slot, which are not supported by the Button component. This will cause the buttons to render incorrectly or not display the text.Lines 334-348 in the second template show the correct pattern for the same functionality.
🔎 Proposed fix
<Button variant="secondary" - label="Settings" @click="() => { close() }" > - <template #icon> - <i class="icon-[lucide--download] size-3" /> - </template> + <i class="icon-[lucide--download] size-3" /> + <span>Settings</span> </Button> <Button variant="primary" - label="Profile" @click="() => { close() }" > - <template #icon> - <i class="icon-[lucide--scroll] size-3" /> - </template> + <i class="icon-[lucide--scroll] size-3" /> + <span>Profile</span> </Button>
| label="Settings" | ||
| <Button | ||
| variant="textonly" | ||
| @click="() => { close() }" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Simplify click handlers.
The arrow function wrappers are unnecessary since close is already a function. You can pass it directly.
🔎 Proposed simplification
- @click="() => { close() }"
+ @click="close"Apply the same change on both lines 27 and 35.
Also applies to: 35-35
🤖 Prompt for AI Agents
In src/components/button/MoreButton.stories.ts around lines 27 and 35, the
@click handlers wrap the existing close function in unnecessary arrow functions;
remove the wrapper and pass close directly (i.e., change @click="() => { close()
}" to @click="close") on both lines to simplify the handlers.
| </template> | ||
| </IconTextButton> | ||
| <i class="icon-[lucide--download] size-4" /> | ||
| <span>Settings</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider using i18n for string literals.
While this is a Storybook story for demonstration, the coding guidelines specify using vue-i18n for all UI strings in src/components/**/*.{vue,ts,js}. Using i18n in stories can serve as a best-practice example and maintain consistency with the guidelines.
As per coding guidelines, all UI strings should use i18n translation keys.
Also applies to: 38-38
🤖 Prompt for AI Agents
In src/components/button/MoreButton.stories.ts around lines 30 and 38, the story
uses raw string literals ("Settings" and another at line 38); replace these with
vue-i18n translation keys: add appropriate keys to your locale file (e.g.
components.moreButton.settings) and update the story to use the i18n translation
function (e.g. this.$t or t('components.moreButton.settings')) or register the
i18n plugin in the storybook preview/decorator so the story renders translated
text; ensure keys are added to locales and the story imports/uses the i18n
instance instead of hard-coded strings.
| <Button | ||
| class="justify-start px-0" | ||
| variant="muted-textonly" | ||
| size="sm" | ||
| icon-position="right" | ||
| @click.stop="copyErrorMessage" | ||
| > | ||
| <template #icon> | ||
| <i class="icon-[lucide--copy] block size-3.5 leading-none" /> | ||
| </template> | ||
| </IconTextButton> | ||
| <IconTextButton | ||
| class="h-6 justify-start gap-2 bg-transparent px-0 text-[0.75rem] leading-none text-text-secondary hover:opacity-90" | ||
| type="transparent" | ||
| :label="t('queue.jobDetails.report')" | ||
| <span>{{ copyAriaLabel }}</span> | ||
| <i class="icon-[lucide--copy] block size-3.5 leading-none" /> | ||
| </Button> | ||
| <Button | ||
| class="justify-start px-0" | ||
| variant="muted-textonly" | ||
| size="sm" | ||
| icon-position="right" | ||
| @click.stop="reportJobError" | ||
| > | ||
| <template #icon> | ||
| <i | ||
| class="icon-[lucide--message-circle-warning] block size-3.5 leading-none" | ||
| /> | ||
| </template> | ||
| </IconTextButton> | ||
| <span>{{ t('queue.jobDetails.report') }}</span> | ||
| <i | ||
| class="icon-[lucide--message-circle-warning] block size-3.5 leading-none" | ||
| /> | ||
| </Button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find the Button component
fd -e vue -e ts "Button" src/components --type f | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1228
🏁 Script executed:
# Search for Button component definition
rg "export.*Button" src/components --type vue --type ts -A 5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Check for icon-position prop usage in the codebase
rg "icon-position" src --type vueRepository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Find Button component in components directory
find src/components -name "Button.vue" -o -name "Button.ts" | head -10Repository: Comfy-Org/ComfyUI_frontend
Length of output: 105
🏁 Script executed:
# Search for Button component export more broadly
rg "export.*Button" src/components --type-listRepository: Comfy-Org/ComfyUI_frontend
Length of output: 459
🏁 Script executed:
# Look for icon-position in all files
rg "icon-position" src --type-listRepository: Comfy-Org/ComfyUI_frontend
Length of output: 459
🏁 Script executed:
# Search for Button component definition and icon-position usage
rg -l "icon-position" src/Repository: Comfy-Org/ComfyUI_frontend
Length of output: 116
🏁 Script executed:
# Check the Button component for icon-position prop
cat src/components/ui/button/Button.vueRepository: Comfy-Org/ComfyUI_frontend
Length of output: 755
🏁 Script executed:
# Also check the full JobDetailsPopover.vue file to understand context
cat src/components/queue/job/JobDetailsPopover.vueRepository: Comfy-Org/ComfyUI_frontend
Length of output: 10860
Remove unsupported icon-position prop from Button components.
The icon-position prop on lines 65 and 75 is not part of the Button component's API and will be silently ignored. The visual layout is already correct because the icon appears after the text in the content order. Remove both instances of icon-position="right".
🤖 Prompt for AI Agents
In src/components/queue/job/JobDetailsPopover.vue around lines 61 to 82, the two
Button components include an unsupported prop icon-position="right"; remove the
icon-position="right" attribute from both Button instances (the copy and report
buttons) so the prop is not passed and the layout relies on the existing content
order that already places the icon after the text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/components/queue/QueueOverlayHeader.vue(1 hunks)src/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
src/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.vue: Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Use setup() function for component logic
Utilize ref and reactive for reactive state
Implement computed properties with computed()
Use watch and watchEffect for side effects
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection
Use vue 3.5 style of default prop declaration
Use Tailwind CSS for styling
Implement proper props and emits definitions
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventions
src/**/*.vue: Vue 3 SFCs must use Composition API only - no Options API
Use<script setup lang="ts">for component logic
Use Tailwind 4 utility classes for styling - avoid<style>blocks
Usecn()utility from@/utils/tailwindUtilto merge Tailwind class names - never use:class="[]"syntax
Never use thedark:Tailwind variant - use semantic values fromstyle.csstheme instead (e.g.,bg-node-component-surface)
Use Vue 3.5+ TypeScript style default prop declaration with reactive destructuring - preferuseModelto prop/emit pairs, do not usewithDefaults
Do not import Vue macros unnecessarily
Use vue-i18n in composition API for string literals - place translation entries insrc/locales/en/main.json
Implement proper props and emits definitions in Vue components
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Avoid new usage of PrimeVue components
Files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
src/**/*.{vue,ts}: Avoid usingrefandwatchtogether - usecomputedinstead if possible
Do not addrefif a prop would suffice; do not addcomputedif the ref/prop directly would work; usewatchonly whencomputedwon't accomplish the goal
Leverage VueUse functions for performance-enhancing styles
Files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
src/**/*.{ts,tsx,vue}: ESLint rules: Vue + TS rules, no floating promises, unused imports disallowed, i18n raw text restrictions in templates
Use es-toolkit for utility functions
Avoid mutable state - prefer immutability and assignment at point of declaration
Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
src/components/**/*.vue
📄 CodeRabbit inference engine (src/components/CLAUDE.md)
src/components/**/*.vue: Use setup() function in Vue 3 Composition API
Destructure props using Vue 3.5 style in Vue components
Use ref/reactive for state management in Vue 3 Composition API
Implement computed() for derived state in Vue 3 Composition API
Use provide/inject for dependency injection in Vue components
Prefer emit/@event-name for state changes over other communication patterns
Use defineExpose only for imperative operations (such as form.validate(), modal.open())
Replace PrimeVue Dropdown component with Select
Replace PrimeVue OverlayPanel component with Popover
Replace PrimeVue Calendar component with DatePicker
Replace PrimeVue InputSwitch component with ToggleSwitch
Replace PrimeVue Sidebar component with Drawer
Replace PrimeVue Chips component with AutoComplete with multiple enabled
Replace PrimeVue TabMenu component with Tabs without panels
Replace PrimeVue Steps component with Stepper without panels
Replace PrimeVue InlineMessage component with Message
Extract complex conditionals to computed properties
Implement cleanup for async operations in Vue components
Use lifecycle hooks: onMounted, onUpdated in Vue 3 Composition API
Use Teleport/Suspense when needed for component rendering
Define proper props and emits definitions in Vue componentsVue components must be named in PascalCase (e.g.,
MenuHamburger.vue)
Files:
src/components/queue/QueueOverlayHeader.vue
src/components/**/*.{vue,css}
📄 CodeRabbit inference engine (src/components/CLAUDE.md)
src/components/**/*.{vue,css}: Use Tailwind CSS only for styling (no custom CSS)
Use the correct tokens from style.css in the design system package
Files:
src/components/queue/QueueOverlayHeader.vue
src/components/**/*.{vue,ts,js}
📄 CodeRabbit inference engine (src/components/CLAUDE.md)
src/components/**/*.{vue,ts,js}: Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners
Use useIntersectionObserver for visibility detection instead of custom scroll handlers
Use vue-i18n for ALL UI strings
Files:
src/components/queue/QueueOverlayHeader.vue
src/**/*.{js,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively - no new JavaScript files
Files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
src/**/*.{ts,tsx,vue,js}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx,vue,js}: Indent with 2 spaces, use single quotes, no trailing semicolons, line width 80 - see.prettierrc
Sort and group imports by plugin, runpnpm formatbefore committing
Files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
🧠 Learnings (26)
📓 Common learnings
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/platform/updates/components/WhatsNewPopup.vue:5-13
Timestamp: 2025-12-09T03:50:03.127Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, when avoiding PrimeVue components, prefer using the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) rather than plain HTML buttons. These components wrap PrimeVue with custom design system styling.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7603
File: src/components/queue/QueueOverlayHeader.vue:49-59
Timestamp: 2025-12-18T21:15:46.862Z
Learning: For Vue components in Comfy-Org/ComfyUI_frontend: Do not suggest adding aria-label to buttons that already have visible text content (e.g., <span> elements with labels). The visible text automatically provides the accessible name. aria-label should only be used for elements without visible labels, such as icon-only buttons.
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue OverlayPanel component with Popover
Applied to files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue InlineMessage component with Message
Applied to files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue InputSwitch component with ToggleSwitch
Applied to files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue TabMenu component with Tabs without panels
Applied to files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Dropdown component with Select
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Sidebar component with Drawer
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-12-09T03:49:52.828Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/platform/updates/components/WhatsNewPopup.vue:5-13
Timestamp: 2025-12-09T03:49:52.828Z
Learning: In Vue files across the ComfyUI_frontend repo, when a button is needed, prefer the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) over plain HTML <button> elements. These components wrap PrimeVue with the project’s design system styling. Use only the common button components for consistency and theming, and import them from src/components/button/ as needed.
Applied to files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
📚 Learning: 2025-12-18T21:15:46.862Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7603
File: src/components/queue/QueueOverlayHeader.vue:49-59
Timestamp: 2025-12-18T21:15:46.862Z
Learning: In the ComfyUI_frontend repository, for Vue components, do not add aria-label to buttons that have visible text content (e.g., buttons containing <span> text). The visible text provides the accessible name. Use aria-label only for elements without visible labels (e.g., icon-only buttons). If a button has no visible label, provide a clear aria-label or associate with an aria-labelledby describing its action.
Applied to files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
📚 Learning: 2025-12-05T06:11:09.383Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7177
File: src/platform/assets/components/UploadModelFooter.vue:72-78
Timestamp: 2025-12-05T06:11:09.383Z
Learning: For the ComfyUI_frontend repository, avoid suggesting comments that would be redundant when the code is already self-explanatory through descriptive naming (e.g., filenames, prop names, aria-labels). The project prefers clean code without unnecessary documentation comments.
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-12-10T03:09:19.636Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:19.636Z
Learning: For test files in the Comfy-Org/ComfyUI_frontend repository: When writing tests, prefer selecting elements by accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests verify actual user-facing behavior and accessibility compliance.
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use vue-i18n for ALL UI strings
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Extract complex conditionals to computed properties
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/{components,composables}/**/*.{ts,tsx,vue} : Use vue-i18n for ALL user-facing strings by adding them to `src/locales/en/main.json`
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use ref/reactive for state management in Vue 3 Composition API
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Implement computed() for derived state in Vue 3 Composition API
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-12-17T01:01:03.609Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T01:01:03.609Z
Learning: Applies to src/**/*.{vue,ts} : Avoid using `ref` and `watch` together - use `computed` instead if possible
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.{vue,ts} : Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize ref and reactive for reactive state
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-12-17T01:01:03.609Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T01:01:03.609Z
Learning: Applies to src/**/*.vue : Use vue-i18n in composition API for string literals - place translation entries in `src/locales/en/main.json`
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-12-09T21:40:12.361Z
Learnt from: benceruleanlu
Repo: Comfy-Org/ComfyUI_frontend PR: 7297
File: src/components/actionbar/ComfyActionbar.vue:33-43
Timestamp: 2025-12-09T21:40:12.361Z
Learning: In Vue single-file components, allow inline Tailwind CSS class strings for static classes and avoid extracting them into computed properties solely for readability. Prefer keeping static class names inline for simplicity and performance. For dynamic or conditional classes, use Vue bindings (e.g., :class) to compose classes.
Applies to all Vue files in the repository (e.g., src/**/*.vue) where Tailwind utilities are used for static styling.
Applied to files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
📚 Learning: 2025-12-16T22:26:49.463Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.vue:17-17
Timestamp: 2025-12-16T22:26:49.463Z
Learning: In Vue 3.5+ with <script setup>, when using defineProps<Props>() with partial destructuring (e.g., const { as = 'button', class: customClass = '' } = defineProps<Props>() ), props that are not destructured (e.g., variant, size) stay accessible by name in the template scope. This pattern is valid: you can destructure only a subset of props for convenience while referencing the remaining props directly in template expressions. Apply this guideline to Vue components across the codebase (all .vue files).
Applied to files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
📚 Learning: 2025-12-18T02:07:38.870Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7598
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:131-131
Timestamp: 2025-12-18T02:07:38.870Z
Learning: Tailwind CSS v4 safe utilities (e.g., items-center-safe, justify-*-safe, place-*-safe) are allowed in Vue components under src/ and in story files. Do not flag these specific safe variants as invalid when reviewing code in src/**/*.vue or related stories.
Applied to files:
src/components/queue/QueueOverlayHeader.vuesrc/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
📚 Learning: 2025-12-18T16:03:02.066Z
Learnt from: henrikvilhelmberglund
Repo: Comfy-Org/ComfyUI_frontend PR: 7617
File: src/components/actionbar/ComfyActionbar.vue:301-308
Timestamp: 2025-12-18T16:03:02.066Z
Learning: In the ComfyUI frontend queue system, useQueuePendingTaskCountStore().count indicates the number of tasks in the queue, where count = 1 means a single active/running task and count > 1 means there are pending tasks in addition to the active task. Therefore, in src/components/actionbar/ComfyActionbar.vue, enable the 'Clear Pending Tasks' button only when count > 1 to avoid clearing the currently running task. The active task should be canceled using the 'Cancel current run' button instead. This rule should be enforced via a conditional check on the queue count, with appropriate disabled/aria-disabled states for accessibility, and tests should verify behavior for count = 1 and count > 1.
Applied to files:
src/components/queue/QueueOverlayHeader.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Steps component with Stepper without panels
Applied to files:
src/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint-and-format
- GitHub Check: test
🔇 Additional comments (2)
src/workbench/extensions/manager/components/manager/button/PackUpdateButton.vue (2)
2-14: LGTM! Past review concern has been addressed.The conditional rendering of the spinner has been correctly implemented with
v-if="isUpdating"on line 12, exactly as suggested in the previous review. The button now shows the spinner only during the update operation, providing clear visual feedback to users.The migration from
IconTextButtontoButtonis well-executed:
variant="textonly"withclass="border"appropriately replaces the old transparent style- Inline content composition (spinner + text) follows the new Button component pattern
- Proper i18n usage and accessibility (visible text label)
- Correct disabled state handling during updates
17-77: LGTM! Script logic is solid.The import update on line 21 correctly aligns with the template migration. The script demonstrates good practices:
- Proper error handling with try-catch-finally
- Edge case handling (empty arrays, missing data)
- Reliable state management with isUpdating reset in finally block
- Clear logging for debugging
- Appropriate filtering for installed packs before updates
| class="w-full justify-start" | ||
| variant="textonly" | ||
| size="sm" | ||
| :aria-label="t('sideToolbar.queueProgressOverlay.clearHistory')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant aria-label.
This button has visible text content (the <span> on lines 57-59), which automatically provides the accessible name. The aria-label attribute is redundant and should be removed. Per accessibility best practices, aria-label should only be used for elements without visible labels, such as icon-only buttons (like the one on line 21-31, which correctly uses aria-label).
🔎 Proposed fix
<Button
class="w-full justify-start"
variant="textonly"
size="sm"
- :aria-label="t('sideToolbar.queueProgressOverlay.clearHistory')"
@click="onClearHistoryFromMenu"
>
<i class="icon-[lucide--file-x-2] size-4 text-muted" />
<span>{{
t('sideToolbar.queueProgressOverlay.clearHistory')
}}</span>
</Button>Based on learnings from previous review feedback on this file.
🤖 Prompt for AI Agents
In src/components/queue/QueueOverlayHeader.vue around line 53, the button
includes a redundant aria-label even though it has visible text in the nested
<span>; remove the
:aria-label="t('sideToolbar.queueProgressOverlay.clearHistory')" attribute from
this button so the visible text serves as the accessible name, leaving
aria-labels only on icon-only buttons (e.g., the one at lines ~21-31) as
appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love to, but it's used to target the element in a test.
This should get cleaned up when we migrate from the basic Vue Test Utils to use Vue Testing Library instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DrJKL, thanks for the explanation! That makes sense. Vue Testing Library facilitates querying the DOM in the same way users would, allowing you to find elements by their label text and ensuring accessibility, so the migration should indeed resolve this.
In the meantime, if it's feasible for the test, you could potentially select this button by its visible text content (the translated "clearHistory" text) instead of the aria-label attribute. That would allow you to remove the redundant aria-label now while still maintaining a reliable test selector. But if the current test structure makes that difficult, keeping it until the Vue Testing Library migration is a reasonable compromise.
Looking forward to seeing this cleaned up in the migration! 🐰
(\\_/)
(•ᴗ•)
/>📚 Planning ahead!
🧠 Learnings used
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7603
File: src/components/queue/QueueOverlayHeader.vue:49-59
Timestamp: 2025-12-18T21:15:46.862Z
Learning: For Vue components in Comfy-Org/ComfyUI_frontend: Do not suggest adding aria-label to buttons that already have visible text content (e.g., <span> elements with labels). The visible text automatically provides the accessible name. aria-label should only be used for elements without visible labels, such as icon-only buttons.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:19.636Z
Learning: For test files in the Comfy-Org/ComfyUI_frontend repository: When writing tests, prefer selecting elements by accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests verify actual user-facing behavior and accessibility compliance.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7177
File: src/platform/assets/components/UploadModelFooter.vue:72-78
Timestamp: 2025-12-05T06:11:09.383Z
Learning: For the ComfyUI_frontend repository, avoid suggesting comments that would be redundant when the code is already self-explanatory through descriptive naming (e.g., filenames, prop names, aria-labels). The project prefers clean code without unnecessary documentation comments.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T01:01:03.609Z
Learning: Applies to tests-ui/**/*.test.ts : For Component testing, use Vue Test Utils and follow advice about making components easy to test
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T01:01:03.609Z
Learning: Applies to tests-ui/**/*.test.ts : For mocking, leverage Vitest's utilities where possible
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Use existing test utilities rather than writing custom utilities
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Implement cleanup for async operations in Vue components
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T01:01:03.609Z
Learning: Applies to tests-ui/**/*.test.ts : Do not write tests dependent on non-behavioral features like utility classes or styles
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Use `test` instead of `it` for defining test cases in vitest
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue TabMenu component with Tabs without panels
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Dropdown component with Select
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue InlineMessage component with Message
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/platform/updates/components/WhatsNewPopup.vue:5-13
Timestamp: 2025-12-09T03:50:03.127Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, when avoiding PrimeVue components, prefer using the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) rather than plain HTML buttons. These components wrap PrimeVue with custom design system styling.
53b6f63 to
d065f50
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/sidebar/tabs/AssetsSidebarTab.vue (1)
129-138: Add aria-label to icon-only buttons for accessibility.The icon-only buttons in compact mode lack aria-label attributes, making them inaccessible to screen reader users who cannot determine the button purpose.
🔎 Proposed fix to add aria-label attributes
<template v-if="isCompact"> <!-- Compact mode: Icon only --> <Button v-if="shouldShowDeleteButton" size="icon" + :aria-label="$t('mediaAsset.selection.deleteSelected')" @click="handleDeleteSelected" > <i class="icon-[lucide--trash-2] size-4" /> </Button> - <Button size="icon" @click="handleDownloadSelected"> + <Button + size="icon" + :aria-label="$t('mediaAsset.selection.downloadSelected')" + @click="handleDownloadSelected" + > <i class="icon-[lucide--download] size-4" /> </Button> </template>Based on learnings, icon-only buttons require aria-label to provide accessible names for assistive technology users.
♻️ Duplicate comments (1)
src/components/widget/layout/BaseModalLayout.stories.ts (1)
210-228: Remove deprecatedlabelprop and#icontemplate slots - inconsistent with second template block.The Button components on lines 210-218 and 220-228 still use the deprecated
labelprop and<template #icon>slots, but the identical buttons in the second template block (lines 334-340 and 342-348) correctly use inline content. The Button component no longer accepts these props after the IconTextButton migration.🔎 Proposed fix to match the second template block pattern
<Button variant="secondary" - label="Settings" @click="() => { close() }" > - <template #icon> - <i class="icon-[lucide--download] size-3" /> - </template> + <i class="icon-[lucide--download] size-3" /> + <span>Settings</span> </Button> <Button variant="primary" - label="Profile" @click="() => { close() }" > - <template #icon> - <i class="icon-[lucide--scroll] size-3" /> - </template> + <i class="icon-[lucide--scroll] size-3" /> + <span>Profile</span> </Button>
| <Button | ||
| v-else | ||
| class="w-full justify-start gap-2 bg-transparent p-2 font-inter text-[12px] leading-none text-text-primary hover:bg-interface-panel-hover-surface" | ||
| type="transparent" | ||
| :label="entry.label" | ||
| class="w-full justify-start bg-transparent" | ||
| variant="textonly" | ||
| size="sm" | ||
| :aria-label="entry.label" | ||
| @click="onEntry(entry)" | ||
| > | ||
| <template #icon> | ||
| <i | ||
| v-if="entry.icon" | ||
| :class="[ | ||
| entry.icon, | ||
| 'block size-4 shrink-0 leading-none text-text-secondary' | ||
| ]" | ||
| /> | ||
| </template> | ||
| </IconTextButton> | ||
| <i | ||
| v-if="entry.icon" | ||
| :class="[ | ||
| entry.icon, | ||
| 'block size-4 shrink-0 leading-none text-text-secondary' | ||
| ]" | ||
| /> | ||
| <span>{{ entry.label }}</span> | ||
| </Button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant aria-label.
The button has visible text content in the <span> element (line 38), which automatically provides the accessible name. The aria-label on line 28 is redundant and should be removed.
Based on learnings, aria-label should only be used for elements without visible labels (e.g., icon-only buttons).
🔎 Proposed fix
<Button
v-else
class="w-full justify-start bg-transparent"
variant="textonly"
size="sm"
- :aria-label="entry.label"
@click="onEntry(entry)"
>📝 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.
| <Button | |
| v-else | |
| class="w-full justify-start gap-2 bg-transparent p-2 font-inter text-[12px] leading-none text-text-primary hover:bg-interface-panel-hover-surface" | |
| type="transparent" | |
| :label="entry.label" | |
| class="w-full justify-start bg-transparent" | |
| variant="textonly" | |
| size="sm" | |
| :aria-label="entry.label" | |
| @click="onEntry(entry)" | |
| > | |
| <template #icon> | |
| <i | |
| v-if="entry.icon" | |
| :class="[ | |
| entry.icon, | |
| 'block size-4 shrink-0 leading-none text-text-secondary' | |
| ]" | |
| /> | |
| </template> | |
| </IconTextButton> | |
| <i | |
| v-if="entry.icon" | |
| :class="[ | |
| entry.icon, | |
| 'block size-4 shrink-0 leading-none text-text-secondary' | |
| ]" | |
| /> | |
| <span>{{ entry.label }}</span> | |
| </Button> | |
| <Button | |
| v-else | |
| class="w-full justify-start bg-transparent" | |
| variant="textonly" | |
| size="sm" | |
| @click="onEntry(entry)" | |
| > | |
| <i | |
| v-if="entry.icon" | |
| :class="[ | |
| entry.icon, | |
| 'block size-4 shrink-0 leading-none text-text-secondary' | |
| ]" | |
| /> | |
| <span>{{ entry.label }}</span> | |
| </Button> |
🤖 Prompt for AI Agents
In src/components/queue/job/JobContextMenu.vue around lines 23 to 39, remove the
redundant :aria-label="entry.label" from the Button element because the visible
<span>{{ entry.label }}</span> already provides the accessible name; if you need
to support icon-only entries in future, add an aria-label only when there is no
visible label (e.g., conditionally bind aria-label when entry.label is absent).
| export const SingleButton: Story = { | ||
| args: { | ||
| variant: 'primary', | ||
| size: 'lg' | ||
| } | ||
| render: (args) => ({ | ||
| components: { Button }, | ||
| setup() { | ||
| return { args } | ||
| }, | ||
| template: ` | ||
| <Button v-bind="args"> | ||
| <i v-if="args.icon === 'left'" class="icon-[lucide--settings]" /> | ||
| {{args.default}} | ||
| <i v-if="args.icon === 'right'" class="icon-[lucide--settings]" /> | ||
| </Button>` | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the actual Button.stories.ts file
find . -name "Button.stories.ts" -path "*/src/components/ui/button/*" | head -1 | xargs -I {} sh -c 'echo "File found: {}"; wc -l {}'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 174
🏁 Script executed:
#!/bin/bash
# Read the actual file content and show lines around 54-67
find . -name "Button.stories.ts" -path "*/src/components/ui/button/*" -type f | head -1 | xargs -I {} sed -n '50,70p' {}Repository: Comfy-Org/ComfyUI_frontend
Length of output: 559
🏁 Script executed:
#!/bin/bash
# Check the Button component definition to understand its interface
find . -name "Button.vue" -o -name "Button.ts" | grep -E "src/components/ui/button" | head -1 | xargs -I {} cat {}Repository: Comfy-Org/ComfyUI_frontend
Length of output: 755
🏁 Script executed:
#!/bin/bash
# Check prettier configuration
cat .prettierrc 2>/dev/null || cat .prettierrc.json 2>/dev/null || echo "No .prettierrc found"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 407
Add spacing in mustache interpolation to comply with formatting standards.
The render function pattern is correct and effectively demonstrates icon usage, but the template has a formatting violation: {{args.default}} should be {{ args.default }} per the project's Prettier configuration which enforces spacing in mustache interpolations.
🤖 Prompt for AI Agents
In src/components/ui/button/Button.stories.ts around lines 54 to 67, the
template uses mustache interpolation without spaces (`{{args.default}}`),
violating the project's Prettier rule; update the template to use spaced
mustache interpolation (`{{ args.default }}`) (and similarly ensure any other
interpolations in this template follow the same spacing convention) so the file
conforms to formatting standards.
| <Button | ||
| variant="secondary" | ||
| @click=" | ||
| () => { | ||
| close() | ||
| } | ||
| " | ||
| > | ||
| <template #icon> | ||
| <i class="icon-[lucide--download]" /> | ||
| </template> | ||
| </IconTextButton> | ||
| <IconTextButton | ||
| type="primary" | ||
| :label="$t('g.profile')" | ||
| <i class="icon-[lucide--download]" /> | ||
| <span>{{ $t('g.settings') }}</span> | ||
| </Button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix icon mismatch for settings button.
The button uses a download icon (icon-[lucide--download]) for a settings action. This is semantically incorrect and may confuse users.
🔎 Suggested fix
<Button
variant="secondary"
@click="
() => {
close()
}
"
>
- <i class="icon-[lucide--download]" />
+ <i class="icon-[lucide--settings]" />
<span>{{ $t('g.settings') }}</span>
</Button>📝 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.
| <Button | |
| variant="secondary" | |
| @click=" | |
| () => { | |
| close() | |
| } | |
| " | |
| > | |
| <template #icon> | |
| <i class="icon-[lucide--download]" /> | |
| </template> | |
| </IconTextButton> | |
| <IconTextButton | |
| type="primary" | |
| :label="$t('g.profile')" | |
| <i class="icon-[lucide--download]" /> | |
| <span>{{ $t('g.settings') }}</span> | |
| </Button> | |
| <Button | |
| variant="secondary" | |
| @click=" | |
| () => { | |
| close() | |
| } | |
| " | |
| > | |
| <i class="icon-[lucide--settings]" /> | |
| <span>{{ $t('g.settings') }}</span> | |
| </Button> |
🤖 Prompt for AI Agents
In src/components/widget/SampleModelSelector.vue around lines 26 to 36, the
settings Button currently uses the download icon class
"icon-[lucide--download]"; change that class to the appropriate settings icon
(e.g., "icon-[lucide--settings]" or your project's lucide settings/gear icon
class) so the visual matches the action, and also update/add an accessible label
(aria-label or title) for the button to reflect "Settings".
| <Button | ||
| variant="primary" | ||
| @click=" | ||
| () => { | ||
| close() | ||
| } | ||
| " | ||
| > | ||
| <template #icon> | ||
| <i class="icon-[lucide--scroll]" /> | ||
| </template> | ||
| </IconTextButton> | ||
| <i class="icon-[lucide--scroll]" /> | ||
| <span>{{ $t('g.profile') }}</span> | ||
| </Button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix icon mismatch for profile button.
The button uses a scroll icon (icon-[lucide--scroll]) for a profile action. This is semantically incorrect and may confuse users.
🔎 Suggested fix
<Button
variant="primary"
@click="
() => {
close()
}
"
>
- <i class="icon-[lucide--scroll]" />
+ <i class="icon-[lucide--user]" />
<span>{{ $t('g.profile') }}</span>
</Button>📝 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.
| <Button | |
| variant="primary" | |
| @click=" | |
| () => { | |
| close() | |
| } | |
| " | |
| > | |
| <template #icon> | |
| <i class="icon-[lucide--scroll]" /> | |
| </template> | |
| </IconTextButton> | |
| <i class="icon-[lucide--scroll]" /> | |
| <span>{{ $t('g.profile') }}</span> | |
| </Button> | |
| <Button | |
| variant="primary" | |
| @click=" | |
| () => { | |
| close() | |
| } | |
| " | |
| > | |
| <i class="icon-[lucide--user]" /> | |
| <span>{{ $t('g.profile') }}</span> | |
| </Button> |
🤖 Prompt for AI Agents
In src/components/widget/SampleModelSelector.vue around lines 37 to 47, the
profile button incorrectly uses the scroll icon class "icon-[lucide--scroll]";
replace that class with an appropriate profile/user icon class (e.g.,
"icon-[lucide--user]" or "icon-[lucide--user-circle]") so the icon matches the
action, and ensure any related styling or accessibility text remains correct
after the change.
d065f50 to
109e3b1
Compare
|
1 similar comment
|
## Summary Replace all the `IconTextButton`s with `Button` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7603-WIP-Component-Button-Migraion-3-IconTextButton-2cd6d73d365081b7b742fa2172dc2ba8) by [Unito](https://www.unito.io)
Summary
Replace all the
IconTextButtons withButton┆Issue is synchronized with this Notion page by Unito