fix(kdropdownitem): add disabled class on links

Kong · Nov 6, 2023 · 2ec91e1 · 2ec91e1
1 parent a708333
commit 2ec91e1
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 18 deletions.
diff --git a/src/components/KDropdown/KDropdown.cy.ts b/src/components/KDropdown/KDropdown.cy.ts
@@ -127,7 +127,7 @@ describe('KDropdown', () => {
     cy.get('.k-dropdown-popover').should('contain.html', itemSlotContent)
   })
 
-  it('correctly renders dividers on all item types', () => {
+  it('correctly renders all item types', () => {
     const itemSlotContent = `
     <KDropdownItem
       @click="() => {}"
@@ -201,13 +201,13 @@ describe('KDropdown', () => {
     cy.getTestId('dropdown-list').eq(0).find('.has-divider').should('have.length', 2)
 
     cy.get('[data-testid="button"] button').should('be.visible')
-    cy.get('[data-testid="disabled-button"] button').should('be.visible').should('be.disabled')
+    cy.get('[data-testid="disabled-button"] button').should('be.visible')
 
     cy.get('[data-testid="router-link"] router-link').should('be.visible')
-    cy.get('[data-testid="disabled-router-link"] router-link').should('be.visible').should('have.attr', 'disabled')
+    cy.get('[data-testid="disabled-router-link"] router-link').should('be.visible')
 
     cy.get('[data-testid="external-link"] a').should('be.visible')
-    cy.get('[data-testid="disabled-external-link"] a').should('be.visible').should('have.attr', 'disabled')
+    cy.get('[data-testid="disabled-external-link"] a').should('be.visible')
   })
 })
 
@@ -239,7 +239,7 @@ describe('KDropdownItem', () => {
     cy.getTestId('dropdown-item-trigger').not(`.${boundClass}`).should('have.length', 1)
   })
 
-  it('correctly handles disabled state', () => {
+  it('correctly handles disabled state on links', () => {
     mount(KDropdownItem, {
       props: {
         item: {
@@ -251,7 +251,26 @@ describe('KDropdownItem', () => {
     })
 
     cy.getTestId('dropdown-item').should('be.visible').should('have.class', 'disabled')
-    cy.get('router-link[data-testid="dropdown-item-trigger"]').should('have.attr', 'disabled')
+    cy.get('router-link[data-testid="dropdown-item-trigger"]').should('not.have.attr', 'disabled')
+    // ensure trigger element has disabled class
+    cy.getTestId('dropdown-item-trigger').should('have.attr', 'class', 'dropdown-item-trigger disabled')
+  })
+
+  it('correctly handles disabled state on button', () => {
+    mount(KDropdownItem, {
+      props: {
+        item: {
+          label: 'You are here',
+        },
+        disabled: true,
+      },
+      attrs: {
+        onClick: () => {},
+      },
+    })
+
+    cy.getTestId('dropdown-item').should('be.visible').should('have.class', 'disabled')
+    cy.get('button[data-testid="dropdown-item-trigger"]').should('have.attr', 'disabled')
     // ensure disabled class doesn't leak to trigger element
     cy.getTestId('dropdown-item-trigger').should('have.attr', 'class', 'dropdown-item-trigger')
   })

diff --git a/src/components/KDropdown/KDropdownItem.vue b/src/components/KDropdown/KDropdownItem.vue
@@ -96,6 +96,7 @@ const type = computed((): DropdownItemType => {
     // checking attrs since we deleted click from listeners
     return 'button'
   }
+
   return 'default'
 })
 
@@ -146,28 +147,27 @@ const availableComponents = computed((): DropdownItemRenderedRecord => ({
   link: {
     tag: 'a',
     attrs: {
-      class: dropdownItemTriggerClass,
+      // add disabled class instead of disabled attribute because disabled is not a valid attribute for links
+      class: `${dropdownItemTriggerClass} ${props.disabled ? 'disabled' : ''}`,
       href: to.value as string,
-      // only add disabled attribute if props.disabled returns truthy value, otherwise it will be added as disabled="false" which will be treaded as disabled
-      ...(!!props.disabled && { disabled: true, tabindex: -1 }),
       ...strippedAttrs.value,
     },
   },
   'router-link': {
     tag: 'router-link',
     onClick: handleClick,
     attrs: {
-      class: dropdownItemTriggerClass,
+      // add disabled class instead of disabled attribute because disabled is not a valid attribute for links
+      class: `${dropdownItemTriggerClass} ${props.disabled ? 'disabled' : ''}`,
       to: to.value,
-      // only add disabled attribute if props.disabled returns truthy value, otherwise it will be added as disabled="false" which will be treaded as disabled
-      ...(!!props.disabled && { disabled: true, tabindex: -1 }),
       ...strippedAttrs.value,
     },
   },
   button: {
     tag: 'button',
     onClick: handleClick,
     attrs: {
+      // don't add disabled class because we want the disabled attribute on the button
       class: dropdownItemTriggerClass,
       disabled: props.disabled,
       ...strippedAttrs.value,
@@ -212,12 +212,12 @@ const availableComponents = computed((): DropdownItemRenderedRecord => ({
     .dropdown-item-trigger {
       color: var(--kui-color-text-danger, $kui-color-text-danger);
 
-      &:hover:not(:disabled):not(:focus):not(:active) {
+      &:hover:not(:disabled):not(.disabled):not(:focus):not(:active) {
         background-color: var(--kui-color-background-danger-weakest, $kui-color-background-danger-weakest);
         color: var(--kui-color-text-danger, $kui-color-text-danger);
       }
 
-      &:focus:not(:disabled), &:active:not(:disabled) {
+      &:focus:not(:disabled):not(.disabled), &:active:not(:disabled):not(.disabled) {
         background-color: var(--kui-color-background-danger-weaker, $kui-color-background-danger-weaker);
         color: var(--kui-color-text-danger, $kui-color-text-danger);
       }
@@ -254,21 +254,21 @@ const availableComponents = computed((): DropdownItemRenderedRecord => ({
       z-index: 1; // need this to prevent box shadow being cut off by the next/previous sibling
     }
 
-    &:hover:not(:disabled):not(:focus):not(:active) {
+    &:hover:not(:disabled):not(.disabled):not(:focus):not(:active) {
       background-color: var(--kui-color-background-neutral-weaker, $kui-color-background-neutral-weaker);
     }
 
-    &:focus:not(:disabled), &:active:not(:disabled) {
+    &:focus:not(:disabled):not(.disabled), &:active:not(:disabled):not(.disabled) {
       background-color: var(--kui-color-background-neutral-weak, $kui-color-background-neutral-weak);
     }
 
-    &:disabled, &[disabled] {
+    &:disabled, &[disabled], &.disabled {
       color: var(--kui-color-text-disabled, $kui-color-text-disabled);
       cursor: not-allowed;
     }
 
     // remove pointer events from only <a>
-    &[disabled]:not(:disabled) {
+    &.disabled {
       pointer-events: none;
     }