From 5337c8914c4a2f53e38e9406f8fbe3694a99bae0 Mon Sep 17 00:00:00 2001 From: "James Rushford (Work)" Date: Thu, 22 May 2025 07:23:16 +0200 Subject: [PATCH 1/2] fix(BaseDropdown): Prevent dropdown interaction while loading instead of disabling button Maintains button's enabled state but prevents dropdown toggling behavior while loading. Improves accessibility by keeping the button focusable --- src/components/base/button/button.spec.js | 11 ++++++++ src/components/base/button/button.vue | 19 +++++++++---- .../base_dropdown/base_dropdown.spec.js | 28 +++++++++++++++++++ .../base_dropdown/base_dropdown.vue | 18 +++++++++++- .../new_dropdowns/listbox/listbox.stories.js | 20 +++++++++++++ 5 files changed, 89 insertions(+), 7 deletions(-) diff --git a/src/components/base/button/button.spec.js b/src/components/base/button/button.spec.js index 7e62fcd1c2..f2e0d46f4f 100644 --- a/src/components/base/button/button.spec.js +++ b/src/components/base/button/button.spec.js @@ -85,6 +85,17 @@ describe('button component', () => { it('should render the loading indicator with the `gl-button-loading-indicator` class', () => { expect(findLoadingIcon().classes()).toContain('gl-button-loading-indicator'); }); + it('should add the disabled class to the button when loading', () => { + expect(wrapper.classes()).toContain('disabled'); + }); + + it('should not remove the button from tab order when loading but not disabled', () => { + expect(wrapper.attributes('tabindex')).toBeUndefined(); + }); + + it('should set aria-disabled attribute when loading', () => { + expect(wrapper.attributes('aria-disabled')).toBe('true'); + }); }); it.each` diff --git a/src/components/base/button/button.vue b/src/components/base/button/button.vue index 262c56e79e..f1a46a1f37 100644 --- a/src/components/base/button/button.vue +++ b/src/components/base/button/button.vue @@ -222,7 +222,7 @@ export default { hasIconOnly() { return isSlotEmpty(this, 'default') && this.hasIcon; }, - isButtonDisabled() { + isDisabledOrLoading() { return this.disabled || this.loading; }, buttonClasses() { @@ -244,6 +244,10 @@ export default { classes.push('btn', 'btn-label'); } + if (this.isDisabledOrLoading) { + classes.push('disabled'); + } + return classes; }, buttonSize() { @@ -270,7 +274,7 @@ export default { }, tabindex() { // When disabled remove links and non-standard tags from tab order - if (this.disabled) { + if (this.disabled && !this.loading) { return this.isLink || this.isNonStandardTag ? '-1' : this.$attrs.tabindex; } @@ -282,14 +286,17 @@ export default { // Type only used for "real" buttons type: this.isButton ? this.type : null, // Disabled only set on "real" buttons - disabled: this.isButton ? this.isButtonDisabled : null, + disabled: this.isButton ? this.disabled : null, // We add a role of button when the tag is not a link or button or when link has `href` of `#` role: this.isNonStandardTag || this.isHashLink ? 'button' : this.$attrs?.role, - // We set the `aria-disabled` state for non-standard tags - ...(this.isNonStandardTag ? { 'aria-disabled': String(this.disabled) } : {}), tabindex: this.tabindex, }; + // We set the `aria-disabled` state for non-standard tags and loading buttons + if (this.isNonStandardTag || this.loading) { + base['aria-disabled'] = this.isDisabledOrLoading; + } + if (this.isLink) { return { ...this.$attrs, @@ -355,7 +362,7 @@ export default { } }, onClick(event) { - if (this.disabled && isEvent(event)) { + if (this.isDisabledOrLoading && isEvent(event)) { stopEvent(event); } }, diff --git a/src/components/base/new_dropdowns/base_dropdown/base_dropdown.spec.js b/src/components/base/new_dropdowns/base_dropdown/base_dropdown.spec.js index 6d249a50a8..f9d0384c94 100644 --- a/src/components/base/new_dropdowns/base_dropdown/base_dropdown.spec.js +++ b/src/components/base/new_dropdowns/base_dropdown/base_dropdown.spec.js @@ -614,4 +614,32 @@ describe('base dropdown', () => { expect(findDefaultDropdownToggle().attributes('aria-labelledby')).toBe(undefined); }); }); + + describe('loading dropdown behavior', () => { + it('should allow closing dropdown when loading', async () => { + // First create the dropdown without loading state + buildWrapper(); + + const toggle = findDefaultDropdownToggle(); + const menu = findDropdownMenu(); + + // Open the dropdown first + await toggle.trigger('click'); + + // Verify it's open + expect(menu.classes('!gl-block')).toBe(true); + expect(toggle.attributes('aria-expanded')).toBe('true'); + + // Now set loading state + await wrapper.setProps({ loading: true }); + + // Attempt to close it + await toggle.trigger('click'); + + // Verify it closes successfully despite loading state + expect(menu.classes('!gl-block')).toBe(false); + expect(toggle.attributes('aria-expanded')).toBe('false'); + expect(wrapper.emitted(GL_DROPDOWN_HIDDEN)).toHaveLength(1); + }); + }); }); diff --git a/src/components/base/new_dropdowns/base_dropdown/base_dropdown.vue b/src/components/base/new_dropdowns/base_dropdown/base_dropdown.vue index e0b5a7c4b4..6a490df933 100644 --- a/src/components/base/new_dropdowns/base_dropdown/base_dropdown.vue +++ b/src/components/base/new_dropdowns/base_dropdown/base_dropdown.vue @@ -28,7 +28,12 @@ import { POSITION_ABSOLUTE, POSITION_FIXED, } from '../constants'; -import { logWarning, isElementTabbable, isElementFocusable } from '../../../../utils/utils'; +import { + logWarning, + isElementTabbable, + isElementFocusable, + stopEvent, +} from '../../../../utils/utils'; import { OutsideDirective } from '../../../../directives/outside/outside'; import GlButton from '../../button/button.vue'; import GlIcon from '../../icon/icon.vue'; @@ -429,6 +434,12 @@ export default { this.stopAutoUpdate?.(); }, async toggle(event) { + // If loading and trying to open the dropdown, prevent and exit + if (this.loading && event && !this.visible) { + stopEvent(event); + return false; + } + if (event && this.visible) { let prevented = false; this.$emit(GL_DROPDOWN_BEFORE_CLOSE, { @@ -499,6 +510,11 @@ export default { this.toggleElement.focus(); }, onKeydown(event) { + if (this.loading && !this.visible) { + stopEvent(event); + return; + } + const { code, target: { tagName }, diff --git a/src/components/base/new_dropdowns/listbox/listbox.stories.js b/src/components/base/new_dropdowns/listbox/listbox.stories.js index 7662a00e51..2603c236e7 100644 --- a/src/components/base/new_dropdowns/listbox/listbox.stories.js +++ b/src/components/base/new_dropdowns/listbox/listbox.stories.js @@ -945,3 +945,23 @@ export const InFormGroup = (args, { argTypes }) => ({ }); InFormGroup.args = generateProps({ toggleId: 'department-picker' }); InFormGroup.decorators = [makeContainer({ height: LISTBOX_CONTAINER_HEIGHT })]; + +export const LoadingDropdown = (args, { argTypes }) => ({ + props: Object.keys(argTypes), + components: { + GlCollapsibleListbox, + }, + data() { + return { + selected: null, + }; + }, + template: template(), +}); + +LoadingDropdown.args = generateProps({ + loading: true, + toggleText: 'Loading Dropdown', + headerText: 'This dropdown is in loading state', + startOpened: false, +}); -- GitLab From 10ef41c90c5c72aecf645624368c954c4b88a830 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 22 May 2025 05:42:21 +0000 Subject: [PATCH 2/2] chore: update snapshots --- ...lapsible-listbox-loading-dropdown-1-snap.png | Bin 0 -> 14152 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/__image_snapshots__/storyshots-spec-js-image-storyshots-base-dropdown-collapsible-listbox-loading-dropdown-1-snap.png diff --git a/tests/__image_snapshots__/storyshots-spec-js-image-storyshots-base-dropdown-collapsible-listbox-loading-dropdown-1-snap.png b/tests/__image_snapshots__/storyshots-spec-js-image-storyshots-base-dropdown-collapsible-listbox-loading-dropdown-1-snap.png new file mode 100644 index 0000000000000000000000000000000000000000..1ed599ee891bf553801fb534a8756c5d90bc3d8d GIT binary patch literal 14152 zcmeAS@N?(olHy`uVBq!ia0y~yU{+vYV2a>iV_;yIRn}C%z@Wh3>Eakt5%>0P<(9jt zwR#`SJ5>}sC5#?CBE?31B?z^Ws%e#-0@!E5*D6vtBbiauwsd8WzQJ(JG-j zck2~B7AB4p0URP3D;~JcOcrwDP*YOm;pE);{`Xu}BKc$)EnySp9W1 zl8^Q1t({OIpy1E|)|_M6ShqR-e56&ug9An-Cobrkw}z{2=wzKYX!x-g?%+yB)|Auf2~B5J7Bu9jW#~IYJR%U$e85dpA$a+{z`VV0 zw|)9>Ci3aS_VhA+^Q!H4PvtFkpU%u*Q+u=ger>BmLN~XDLbMGNGt3F6*hD9!&#T;4 z^V0kIl-v3L&zlDA`&Y_7#oqR3bL#80+ui&Ux;ZrzqI>Rf3qu_;jq}T{hAaF5`qSeJ zQ-A)tU%zoK1fzq@?({{MgWTJ5rB z5ihT=m#<@rc5z_5sq`jp3OFhR6gD(Dtmi(lr?U959&6 zIg_r(+@7qLzhT>Z-~YRqCe2&$lM@_7jr$ZXtY+F&@Nn~2=ey5Je)mgRZsL)#$mkX0 z7S~&I|NjfA{q^4B6UtoDKRpef5`N{=#@Bkgb0+2N|MPVH6!HBPvx|N_6yCMisrdQd zO?OHjPx%28t2pX~+Yupxuv!=Yev-9c7<@2?g*{>^0 zS+D!maqQH3^Za{zs*a=_%&JfG7N2nTqx7a#vf}!2F?u^5yg$7m(R%9k{eN?3C-;cP zIWyjT^CWHpG+;m2M}dsIrp!P6X!rCf6P4YkIx@4b6HYiEIAQI!TdUp}z1rSyux4$; z7G<{Z*jnG2zvc(Oei_7ldPm{oQy1OkL)-Y}ukEd~ZYq4-Yd$qPfA3zKk4J>R-fy4I znQyt|jcMKb)X#T+yf>dZdzp`>zwOtO%AcqIhu>T`kLl|LXZ!bl2ebP9>wmrZ#9m(z zb2EK@?WzBN&)H9xEx)tSCR%^X{?F&EcQ3YSzw$cfoBe~Wr{}#t$S$ulS%3GUGnH>P z8tuCG@%7pKy5yZ74%KVS|9$7?YLm#jefED}oJPOBQEj(>(3?}Y+`*nNj@g(-P-lm2?rXg*f;Mz9q29_$;K-cqW5ch zz3zG2?=dsKoU?!PZTIte)w^r`Egs%YJ#YK{#lN@F_kWe$tZRRJ`n|k);ar~UUrv8+ z)&JvLdGqwQwb%K4BOcyf_vc~g({u9oL+yTD>|b;FecX9-OV;eYJ8Fy}8Q&pcGRK!y z4QFOpF0MXoUv}n3soW~jj+tM@_P)K;U$5JJir4&3gx$x^{<8S|qbZ-B zoJ>usO}?WyW&i7S^QYX-+q+h5s^Dr}1^zqr^_Gu1lu!NmSY39%{$}d*>HM`n&YwCV zsC?>9{2ATnf8OqV+Q9s6eg3cbK)K44vYp>vn0|FRyv=FG$AJd{lipdHx@l>B*mW{CpODO8V8>Y5g{jK5R-kIjQK`%=D?A z^*{R7No=*fma=18{JgLw>-K*8^=Vnu8@~UgoS9D_|9YJl@$mk&Z#G;fc5(f*gB9e< z#BVHMy3}=K*S)K&M5l$tR0TWCt9&-I=vw4_&G(h(KYr!?#$BJ8fByPEr#c5)rNRe| z`7i#}|9)M2{^*^z90%0xf7YMstp9TFlePZOxkcxu|6BJa`2Vc`(|#S>_57k=ZPw>C z?>J5rcZ-$Q-+f>C=W6`l(#pqF!xvN*nz2)$=UYc3U?J@iB zV|n|%vy;EyEq{M+hkk8crscmMo~P&izEgaDed-g{ufHbVk6BWG(e=Lm+C6)I-#uQ= z`s9+g{_DO)U+%oy5xe~Iw|^(x{dBG0@3|bs`|jsW+q>EG%WfUddU<16_1m(GHM=YR z`TN_gd=rzeaG~pxr~DrW$-S@F-TnM_@A)55Jn!|@udnx?ZML@Nv9$O!<8u~!-GP?ulwS=#1($& zZ0SDKZ~N>;>iY6~mEu$We!IO~CdB*1_5J_W8WmmXC@QhFUHSaty#w(--qy#wXS(Fa zY5jW5=IOGrnfZ6NuU=n!W_|o_(J5yCKmM-{lHC9ON9y|W*SXuTA1cn()t7(Ebz(|Y z)y?=nN9XJRuKK;x_?*S&J&$Gcb*ta+{T{^|Rk`c;yZO^5|9{i0edP1Z^EK;aqPlOt zziK^mtIW4m%cJ-HGF`vw=#%cJ$@0HXf4X_TZr_}`U&nU`KbV#7pZVHPHM;rXu76hF zr%d?vcdxGe+jP+hrJ~k!&-}m!u{`zvpoxk7hK3%@QI#8E;x$bVZl&P#s z#8!Mf9eeqb-^#4zwO>UhNayWX`1`(Xf0ur7(~ztoqe^WWn4 zoAagH)$>@F%>DNz!1mI|SK<3t{bo2-mUWF)yk_m5m%Z#$U%%GO*RQ@=_jTKu-{toE ziY7+gEU2wpd;4X?Tf@Xd(bt|I5Bm|vy5xFE9js#8aFL}d)Zx}w*8VefOQe%x^<3ir zZQHy1-KOmC2V22OX~}buHNt)s;pM(A)m!$ztN++tAN;L8*OvA50`n`EH~!ltv*G{0 zyI(opeta^S`_#?!`m&p>m+GU}U%#=r?wQ;9tt@Y=&;36Ye!AfwGsvc`0{wpuoLdE??^XNvCq6xXq4&CX@BU+rG7FStKy%VJ5}<%PQS zTw&TVo73&1xQ~aQ)_zs0Z~bnAbGgY2_v)`@>) zb}Rp*mEA<9O??6TdZ9J{22NIxvu$j*U(HQT`my2R-S~ImUo)0x*@nFrx~dnDcRl^h zj>Fvfe&Kann2grME&s6Vy8MPi)5~sUKCilU^+3@_e!Z7^d-9(D+kX51w#BmFuW)a* z|MM_-SNF2pccq~I=|j(#9m%*mA(F=-|Xz`m%iBC(hNUuw|hg)1OIqh%h@csXH}MAyUTFF z>=k<%=ly^3=iQ+X-!isu%*_uA|DM5`^6SgX{a>G!wJeNvxp%j1yIkxHtFL|U%=RQ7 z?^}J{UdkQS$qAD#J>&)EAQqO#mAox-aYr-tGwdor?z8&Udi%pSn_G9c z)_t1%|9b3)dvdz7%hr1CcK!J2`VkEU>2LDO=c_?GQD0m^VK&cvUh(s@x^G2Rn%Zt# zyZvCUyV13;eUX|M-fFo0Ueh!0_};H!JMXV#eR6sJzbk3+3uPHIS)WV^_Ph6H{oGGY zeGN-gQvD!R^a6eEoB+m`jhWl`)UJw4_;+UW`Dw1*Vr8z2=Wg44_S)?$mltn~edt>i zf3T+F+M1$=+y5<;3DkGGUwh}qr`1&Icc>84LLLB&*u{gaA-PYqy=fgT}TIY-K5tZ z4HFA+_~QpIuo;;Qp4(%hUU$ELZu*Y3Nll z*)qNDOWpiBck%IFY35dFUUo>}=K8W)e!=#V)oazk zez~!6@oxU`FR<>1z>OlNzpvD1>F4HOdUmFAP2Hg%i{1sVwdMFC+8AcBZQcAwb~Q%~ zmP>w{8t@jH4P3ZOSXHW+55qc#pgy_sf`%h5sR58E0_B#UOk5syZDw$56a+?fziYO4gpY5jFt@GpcpL~ zMiT=l1V$4BI0QgJF=nXvr{|7(gK~k`u#!`J?-m-|}p65@cXtVDNPH Kb6Mw<&;$UMxgPWY literal 0 HcmV?d00001 -- GitLab