From 8f9fe3b21eca1545794bff0c1a0b94b0f2806831 Mon Sep 17 00:00:00 2001 From: Dante Date: Tue, 24 Mar 2026 13:39:20 +0900 Subject: [PATCH] refactor: rebuild SingleSelect and MultiSelect with Reka UI (#9742) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Rebuild `SingleSelect` with Reka UI Select primitives (SelectRoot, SelectTrigger, SelectContent, SelectItem) - Rebuild `MultiSelect` with Reka UI Popover + Listbox (PopoverRoot, ListboxRoot with `multiple`) - Remove PrimeVue dependency from both components - Preserve existing API contract for all consumers ## TODO - [x] Re-evaluate MultiSelect implementation (ComboboxRoot with `multiple` may be cleaner than Popover+Listbox) - [x] E2E verification in actual app (AssetFilterBar, WorkflowTemplateSelectorDialog, etc.) ## Test plan - [x] Storybook visual verification for all SingleSelect/MultiSelect stories - [x] Keyboard navigation (arrow keys, Enter, Escape, typeahead) - [x] Multi-selection with badge count - [x] Search filtering in MultiSelect - [x] Clear all functionality - [x] Disabled state - [x] Invalid state (SingleSelect) - [x] Loading state (SingleSelect) ## screenshot 스크린샷 2026-03-20 오전 12 12 37 스크린샷 2026-03-20 오전 12 23 51 ## video https://github.com/user-attachments/assets/2fc3a9b9-2671-4c2c-9f54-4f83598afb53 Fixes #9700 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9742-refactor-rebuild-SingleSelect-and-MultiSelect-with-Reka-UI-3206d73d36508113bee2cf160c8f2d50) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Opus 4.6 Co-authored-by: Alexander Brown --- browser_tests/tests/templates.spec.ts | 25 ++ ...r-bar-select-components-chromium-linux.png | Bin 0 -> 4543 bytes src/components/input/MultiSelect.test.ts | 60 +++ src/components/input/MultiSelect.vue | 393 +++++++++--------- src/components/input/SingleSelect.vue | 237 +++++------ src/locales/en/main.json | 3 +- 6 files changed, 376 insertions(+), 342 deletions(-) create mode 100644 browser_tests/tests/templates.spec.ts-snapshots/template-filter-bar-select-components-chromium-linux.png create mode 100644 src/components/input/MultiSelect.test.ts diff --git a/browser_tests/tests/templates.spec.ts b/browser_tests/tests/templates.spec.ts index 9ae07c79fb..516f368a5f 100644 --- a/browser_tests/tests/templates.spec.ts +++ b/browser_tests/tests/templates.spec.ts @@ -206,6 +206,31 @@ test.describe('Templates', { tag: ['@slow', '@workflow'] }, () => { await expect(nav).toBeVisible() // Nav should be visible at tablet size }) + test( + 'select components in filter bar render correctly', + { tag: '@screenshot' }, + async ({ comfyPage }) => { + await comfyPage.command.executeCommand('Comfy.BrowseTemplates') + await expect(comfyPage.templates.content).toBeVisible() + + // Wait for filter bar select components to render + const dialog = comfyPage.page.getByRole('dialog') + const sortBySelect = dialog.getByRole('combobox', { name: /Sort/ }) + await expect(sortBySelect).toBeVisible() + + // Screenshot the filter bar containing MultiSelect and SingleSelect + const filterBar = sortBySelect.locator( + 'xpath=ancestor::div[contains(@class, "justify-between")]' + ) + await expect(filterBar).toHaveScreenshot( + 'template-filter-bar-select-components.png', + { + mask: [comfyPage.page.locator('.p-toast')] + } + ) + } + ) + test( 'template cards descriptions adjust height dynamically', { tag: '@screenshot' }, diff --git a/browser_tests/tests/templates.spec.ts-snapshots/template-filter-bar-select-components-chromium-linux.png b/browser_tests/tests/templates.spec.ts-snapshots/template-filter-bar-select-components-chromium-linux.png new file mode 100644 index 0000000000000000000000000000000000000000..4a39a259b3445c7b64218aae22cbddda223b1b56 GIT binary patch literal 4543 zcma)=cQl)A_{U>!TC?Rf%4=4rP1LBp_o&&@fz+xBC4^E^YSf5Ysag?DQN%2w)Tk8| znre({o7$rkMevLFJ-_q&>pkc9&vV`9xzBaq&$;gF`99yzlVWFM%E2ng3IG5&%*~7( z006p|)UhTrE%h20P3sH*oCBB}U2}R^yf(|?X}x^PwB@KVYay3Q_RGf!T2kbveWe=G zs_ib_Y8bmIll+0(L?XSWueSd9vC^KT0~%Xe_KZ9ES+y1A*!H*nkRAXic zNW$_CENYh6hS%B;*I9JyT8tXga>WD??y?hA)r`OR21WCws4gRD&`1l5+sbY%ofeV~ z(KVm^?AdY+=eFT;1GJM|WYNc^C=U^iF*?usjN0q+pT@^;0v?xPu}%&~teER>?Acy% zMh%Nj>F~dV$()K^Rh!%nz4&#at-|~GP&|vDPRyv`+qZ;-7|^^^0|)pW{My{MsdEMc zT69Q{S!fZMDbvtQuyE_dcLK5A#e4e#X>=CzUR^Z!QJ^fL`y>9WCYFX~p zr7M60lceH)dpVD%?k`z?0OMrz{@K<}Ae6Kn+AI-NuU^Ya`>x!e7Rc|~%-^KXH|TL< z2b0%7wmT_R_}WZf2<)PBl(sXKX$!j8q;E`}?Os3VNZ96r(8GaTeh|hTH#^fOMhF+Z zNm{;3AqR#Z>H)ik2XIhAZypnkg8r^drwwnDz9cnKKY53hlWFq&U;L1fXzE!4;$Mn& zGn@^sU5jTsiwOw;fwM@@K%;OLnNy#IjC7n6U0tI%-?=BG5ga4E?n12f%769CyG>l| zs~+}>@B6Lc=iCnuDiRq*e~_{$8wx!SK#to=aI|n-*>^>$Ge%j zhv>jmE2-7-dcs`nQQx<1T=v>G(~lV=F2uuF)kfWb9Sh!0lUxokTqlf@N?GoWsJfFpzeG2~KhHcdg=ubXPHnL+R9&8K99Sm>q~>0uV`5}t^0Ku~ zNy#YGj^sR&n05L_taU@isL3kq&7>{YIz5_Bm*w95<$Y09peVZ-yGWcxp-@Ao(Gh#S z=q3L?+*1Ap)QBYO>%-ku!kmBDqJuB3cEChk$UM)rfzNfS_dZ!gs0c#5aj^BSPH3oe z;+On_f`}_2r@UF3&AAjgy# z35lMZTF);{4+a(!84;wSCWxL~UTpL}Y%_0DGQI=2xJ)RX#ciZKd4k;t869M*+^CpU z*3j@HI!tl0=@_&1CUfWr@uytB4oZxM~y9oMZgzdQfZ{3(inDB zH7bKwe2)u?+1n%;k{XvUU6S@+)Q+U5XB5$jmAk$56<+}6o}<2N{?ps#2|Yb8dET|P z;YD?$!^Jfpa~Emc>qp0%1XVve48uMM4&L}Cs`L51OO5--+fLh4mste96JL89L*5Mz zZuRfp!=_s3LQ{HXv>K;jJ6olRv4<3`Ambf$jBUo(!px~Ku3TgQ?-ndfkA+e@1rsTB-ahkq({x;@U>Huf40!^ceVF7NOi3p;MF|qsHmuN zoS-365qV_qH8>2Nu72R^u0|$R*Ao23f28CP%tA!xFBz^kiroGv>+1|dL6#@T0*X?9 zX@z0^>gL$dUO;fvp9^0dB%^j$oC5=A2D5snd9}6Qj*nmIW3-Vly$Yi|oNeFL(!%>^ zi&L!LUpewgi>Z(kSI(xLhjWSAWe1FBvBm~^UQKe#_4f_5N}|klLRFtM_~GY7bb#|6QI2w^j*mRlLqHF=?7r6D zLIkh0=88T{9VP6Vw9bi*Pr363Z5}@kv}rrfE$bCNZHCB zogZ2+HH6czeU73Q#WYjo;LjgHA))=}Tzok@_4oHTDhagY7p0{>Px*oQE9XMB;9o9F zNF<^htUX##q44A~nCDu-ix(oIaXtjpsA2MF1S#L95ra@AL~KvWW*X}~gdR1A5NoO4f`yawqUKG&+tlcUc@7V^+Gkv+6Pf*@rD zyv9R>D%j_{Okt{iQU)la#(gP&^VyI~zYK&q0M%x%ndTGm_H>)siJDt1?% zS2T_=uXLzy&X3Y2wBP&X%LS^e+*!%Z&299 z_#%JF?Rr%&FJV;SWB0MrJ>L+Q@_l1;|(a^b(cEQM z)sVU3i2(1OcRrdmO&i3>vA;B|t*zf)3XxW2XJ@bA4j`bc;o6`#&!?SRCHufuYAj+Q z#Mf>`Sy|?6G>~(8Jy`(qXpWw2d1sDE{R)1 zT9aVm5$*Adw>~8@@z2Wpr&W9GzaXp22=zoq0hLtL$^)h_A3j7!xBt4&cU9bEca^17 zYBDtsj+1CrE|6E1&qZ-Ue=l_f9;7y4M08{Pi3g`4*N)zgq@hmzFKK8HD^ggB%yJ~w zyu?%?W?s+F#5dkjm@-e#QRo{Rd6LzyF@e4{{&pB)DJzh)}6VV-S zBj9dy#k9eCP4<9%w<4IIG3qVhq6~xg>!7t#UK?}ALG~kf#n2+1fe!%{@PV=l%m*i< z%QMQC77T;qUav<%cYcPfIr3&ZIv5z*jz*TWOpxGn{Db(re6HDQ0TV_CFn=rLU9y5_ ztJ!IGjw6KP${fHZd{Dy&eJG z+55@FDUz1{w46+{9dMmL-EWF;sG{j(`~4C8&;0h*AzUOwR8Pr&TptLm341;utSw@c zG&i~q<>9%-1ciEd2wC|2AY!#NHLvfln*JguFl?+zvO0Ms8rs+#{uH{5!7q7+_B{S| zkNA3mfSGu3k~{q2gU=5EX{`URy6#&8)xoDr^-ma+Wz#-4LmJaZ1=uMB^fwk1F)r&l z@#QtT!tdo<_uj?qyVl*PZ?RYK)GX=etUi57J~^(b?rs!_8NRI=C8izrLx$l2=Eoz~ zJ07qLK0koSB!wGYzLY=rJAuH92j@{J8+0J_0IEf^!r1WusbTmYM-xXW)bL_$C8>W-aBQ+Z&DE9ZDkF>NjEAQPH zjhy6Fe7{l9d{=tc&bvhT1;B=piHSX0kULC#*S!H{hV|_mVq|1w+S{aB=;_88zelEA z^X6}xeoHG-zo21Ur4*kmQ&Mw~mnW(-a=1OkcN~rO7cD`o6=yDTD?*u$y1ON)UBJo4 zML<(fVW7Cs$P!zf-${QyV@bH(Rbt3;1DV`hI$bq6>2#XDnwTLi2JL2E+1XcKz8c z=1>~yU46TAU(d+=zhN$^aq(-{kx&~Tp`rp^&W~oM((hoX_q&C7DkPSTxy4`3;Zp%X z;LN947{4aj~A4SfjeBcKI~tw5un(VWBf^cYCavV$MV zQQ5haay|aAlw#=B9dyRdmVX-?_w}{fFw;LxZE$xB<(g%d<9#vyOLc{jR1RVs<0M{51jkYtT#H%*bl5)(e(eIdom`#uFBX3- zNw}d*N3HdL12ZeF=h^%@Ui#LvC?@m&J*597?*CligbM&TJsq#(52voncvf>dfRAsR U4$u2e)Y(?%#x_Q^1~(J_2k?=wVgLXD literal 0 HcmV?d00001 diff --git a/src/components/input/MultiSelect.test.ts b/src/components/input/MultiSelect.test.ts new file mode 100644 index 0000000000..fda3095de8 --- /dev/null +++ b/src/components/input/MultiSelect.test.ts @@ -0,0 +1,60 @@ +import { mount } from '@vue/test-utils' +import { describe, expect, it } from 'vitest' +import { nextTick } from 'vue' +import { createI18n } from 'vue-i18n' + +import MultiSelect from './MultiSelect.vue' + +const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: { + en: { + g: { + multiSelectDropdown: 'Multi-select dropdown', + noResultsFound: 'No results found', + search: 'Search', + clearAll: 'Clear all', + itemsSelected: 'Items selected' + } + } + } +}) + +describe('MultiSelect', () => { + function createWrapper() { + return mount(MultiSelect, { + attachTo: document.body, + global: { + plugins: [i18n] + }, + props: { + modelValue: [], + label: 'Category', + options: [ + { name: 'One', value: 'one' }, + { name: 'Two', value: 'two' } + ] + } + }) + } + + it('keeps open-state border styling available while the dropdown is open', async () => { + const wrapper = createWrapper() + + const trigger = wrapper.get('button[aria-haspopup="listbox"]') + + expect(trigger.classes()).toContain( + 'data-[state=open]:border-node-component-border' + ) + expect(trigger.attributes('aria-expanded')).toBe('false') + + await trigger.trigger('click') + await nextTick() + + expect(trigger.attributes('aria-expanded')).toBe('true') + expect(trigger.attributes('data-state')).toBe('open') + + wrapper.unmount() + }) +}) diff --git a/src/components/input/MultiSelect.vue b/src/components/input/MultiSelect.vue index 813bb46929..74a2b23f8b 100644 --- a/src/components/input/MultiSelect.vue +++ b/src/components/input/MultiSelect.vue @@ -1,207 +1,215 @@ diff --git a/src/locales/en/main.json b/src/locales/en/main.json index 853bb58c3f..c648bcdd7b 100644 --- a/src/locales/en/main.json +++ b/src/locales/en/main.json @@ -278,8 +278,7 @@ "clearAll": "Clear all", "copyURL": "Copy URL", "releaseTitle": "{package} {version} Release", - "itemSelected": "{selectedCount} item selected", - "itemsSelected": "{selectedCount} items selected", + "itemsSelected": "No items selected | {count} item selected | {count} items selected", "multiSelectDropdown": "Multi-select dropdown", "singleSelectDropdown": "Single-select dropdown", "progressCountOf": "of",