fix(components): [el-dropdown] cannot be closed by clicking outside (#5287)

- Fix the issue that dropdown with trigger 'click' cannot be closed when clicking outside content
- Fix the same issue for popover popconfirm
- Remove useless code from `el-tooltip-content` which can be much simpler
- Use `onClick` to replace `onMousedown` because `onMousedown` is triggered prior than `onClick`
- Adjust test cases against these changes above

packages/components/dropdown/__tests__/dropdown.spec.ts

@@ -9,7 +9,6 @@ import DropdownMenu from '../src/dropdown-menu.vue'
 
 const MOUSE_ENTER_EVENT = 'mouseenter'
 const MOUSE_LEAVE_EVENT = 'mouseleave'
-const MOUSE_DOWN = 'mousedown'
 const CONTEXTMENU = 'contextmenu'
 
 jest.useFakeTimers()
@@ -144,7 +143,7 @@ describe('Dropdown', () => {
     jest.runAllTimers()
     await rAF()
     expect(content.open).toBe(false)
-    await triggerElm.trigger(MOUSE_DOWN, {
+    await triggerElm.trigger('click', {
       button: 0,
     })
     jest.runAllTimers()

packages/components/dropdown/src/dropdown-menu.vue

@@ -1,7 +1,7 @@
 <template>
   <ul
     :ref="dropdownListWrapperRef"
-    :class="['el-dropdown-menu', size && `el-dropdown-menu--${size}`]"
+    :class="dropdownKls"
     :style="rovingFocusGroupRootStyle"
     :tabindex="-1"
     role="menu"
@@ -14,8 +14,7 @@
   </ul>
 </template>
 <script lang="ts">
-import { defineComponent, inject, unref } from 'vue'
-import { ClickOutside } from '@element-plus/directives'
+import { computed, defineComponent, inject, unref } from 'vue'
 import { EVENT_CODE } from '@element-plus/utils/aria'
 import { FOCUS_TRAP_INJECTION_KEY } from '@element-plus/components/focus-trap'
 import {
@@ -35,10 +34,6 @@ import { useDropdown } from './useDropdown'
 
 export default defineComponent({
   name: 'ElDropdownMenu',
-  directives: {
-    ClickOutside,
-  },
-  inheritAttrs: false,
   props: dropdownMenuProps,
   setup(props) {
     const { _elDropdownSize } = useDropdown()
@@ -70,6 +65,10 @@ export default defineComponent({
       undefined
     )!
 
+    const dropdownKls = computed(() => {
+      return ['el-dropdown-menu', size && `el-dropdown-menu--${size}`]
+    })
+
     const dropdownListWrapperRef = composeRefs(
       contentRef,
       dropdownCollectionRef,
@@ -120,6 +119,7 @@ export default defineComponent({
       size,
       rovingFocusGroupRootStyle,
       tabIndex,
+      dropdownKls,
       dropdownListWrapperRef,
       handleKeydown,
       onBlur,

packages/components/popconfirm/__test__/popconfirm.spec.ts

@@ -37,7 +37,7 @@ describe('Popconfirm.vue', () => {
       'display: none'
     )
 
-    await trigger.trigger('mousedown', {
+    await trigger.trigger('click', {
       button: 0,
     })
 

packages/components/popover/__tests__/directive.spec.ts

@@ -55,7 +55,7 @@ describe('v-popover', () => {
     expect(
       document.body.querySelector('.el-popover').getAttribute('style')
     ).toContain('display: none')
-    await wrapper.find(refNode).trigger('mousedown', {
+    await wrapper.find(refNode).trigger('click', {
       button: 0,
     })
     await nextTick()

packages/components/popper/__tests__/trigger.spec.ts

@@ -57,19 +57,19 @@ describe('<ElPopperTrigger />', () => {
 
   describe('can attach handlers', () => {
     it('should be able to attach handlers to the trigger', async () => {
-      const onMousedown = jest.fn()
+      const onClick = jest.fn()
       const virtualRef = document.createElement('div')
       wrapper = mountTrigger({
-        onMousedown,
+        onClick,
         virtualTriggering: true,
         virtualRef,
       })
       await nextTick()
-      expect(onMousedown).not.toHaveBeenCalled()
-      const evt = new MouseEvent('mousedown')
+      expect(onClick).not.toHaveBeenCalled()
+      const evt = new MouseEvent('click')
       virtualRef.dispatchEvent(evt)
       await nextTick()
-      expect(onMousedown).toHaveBeenCalled()
+      expect(onClick).toHaveBeenCalled()
     })
   })
 })

packages/components/popper/src/trigger.vue

@@ -24,7 +24,7 @@ export default defineComponent({
     ...usePopperTriggerProps,
     onMouseenter: Function,
     onMouseleave: Function,
-    onMousedown: Function,
+    onClick: Function,
     onKeydown: Function,
     onFocus: Function,
     onBlur: Function,
@@ -55,7 +55,7 @@ export default defineComponent({
           ;[
             'onMouseenter',
             'onMouseleave',
-            'onMousedown',
+            'onClick',
             'onKeydown',
             'onFocus',
             'onBlur',

packages/components/time-picker/__tests__/time-picker.spec.ts

@@ -371,7 +371,7 @@ describe('TimePicker', () => {
     await nextTick()
     const popperEl = document.querySelector('.el-picker__popper')
     const attr = popperEl.getAttribute('aria-hidden')
-    expect(attr).toEqual('true')
+    expect(attr).toEqual('false')
   })
 
   it('model value should sync when disabled-hours was updated', async () => {

packages/components/tooltip/__tests__/content.spec.ts

@@ -1,6 +1,6 @@
 import { nextTick } from 'vue'
-import { shallowMount } from '@vue/test-utils'
-import ElTeleport from '@element-plus/components/teleport'
+import { mount } from '@vue/test-utils'
+import { usePopperContainer } from '@element-plus/hooks'
 import { genTooltipProvides } from '../test-helper/provides'
 import ElTooltipContent from '../src/content.vue'
 import { TOOLTIP_INJECTION_KEY } from '../src/tokens'
@@ -34,26 +34,43 @@ describe('<ElTooltipContent />', () => {
     },
   }
 
-  const createComponent = (props = {}, provides = {}) =>
-    shallowMount(ElTooltipContent, {
-      props,
-      global: {
-        provide: {
-          ...defaultProvide,
-          ...provides,
+  let unmount
+  const createComponent = (props = {}, provides = {}) => {
+    const wrapper = mount(
+      {
+        components: {
+          ElTooltipContent,
+        },
+        template: `<el-tooltip-content><slot /></el-tooltip-content>`,
+        setup() {
+          usePopperContainer()
         },
       },
-      slots: {
-        default: () => [AXIOM],
-      },
+      {
+        props,
+        global: {
+          provide: {
+            ...defaultProvide,
+            ...provides,
+          },
+          stubs: ['ElPopperContent'],
+        },
+        slots: {
+          default: () => [AXIOM],
+        },
 
-      attachTo: document.body,
-    })
+        attachTo: document.body,
+      }
+    )
+
+    unmount = () => wrapper.unmount()
+    return wrapper.findComponent(ElTooltipContent)
+  }
 
   let wrapper: ReturnType<typeof createComponent>
   const OLD_ENV = process.env.NODE_ENV
   beforeAll(() => {
-    process.env.NODE_ENV = 'development'
+    process.env.NODE_ENV = 'test'
   })
 
   afterAll(() => {
@@ -64,7 +81,8 @@ describe('<ElTooltipContent />', () => {
     ;[onOpen, onClose, onToggle, onShow, onHide].forEach((fn) => fn.mockClear())
     open.value = false
     controlled.value = false
-    wrapper?.unmount()
+    trigger.value = 'hover'
+    unmount?.()
     document.body.innerHTML = ''
   })
 
@@ -77,31 +95,6 @@ describe('<ElTooltipContent />', () => {
     })
 
     describe('persistent content', () => {
-      it('should teleport the content to the body when teleport is not disabled', async () => {
-        wrapper = createComponent({
-          persistent: true,
-        })
-        await nextTick()
-
-        const teleportComponent = wrapper.findComponent(ElTeleport)
-        expect(teleportComponent.props('disabled')).toBe(false)
-      })
-
-      it('should not teleport the content to body when teleport is disabled', async () => {
-        wrapper = createComponent({
-          persistent: true,
-          teleported: false,
-        })
-        await nextTick()
-
-        const teleportComponent = wrapper.findComponent(ElTeleport)
-        expect(teleportComponent.props('disabled')).toBe(true)
-        const { vm } = wrapper
-        expect(vm.shouldRenderTeleport).toBe(true)
-        expect(vm.shouldRenderPopperContent).toBe(true)
-        expect(vm.shouldShowPopperContent).toBe(false)
-      })
-
       it('should be able to inherit style', async () => {
         const customStyle = {
           position: 'absolute',
@@ -116,121 +109,15 @@ describe('<ElTooltipContent />', () => {
       })
     })
 
-    describe.only('displaying content when non-persistent', () => {
-      it('should be able to show and hide content when updating the indicator', async () => {
-        wrapper = createComponent()
-        await nextTick()
-
-        const { vm } = wrapper
-        // when non persistent this should always be true so we only assert it once
-        expect(vm.shouldShowPopperContent).toBe(true)
-
-        expect(vm.shouldRenderTeleport).toBe(false)
-        expect(vm.shouldRenderPopperContent).toBe(false)
-
-        open.value = true
-
-        expect(vm.shouldRenderTeleport).toBe(false)
-        expect(vm.shouldRenderPopperContent).toBe(false)
-        // for allowing vue to trigger update effects
-        await nextTick()
-        expect(vm.shouldRenderTeleport).toBe(true)
-        expect(vm.shouldRenderPopperContent).toBe(false)
-        await nextTick()
-        expect(vm.shouldRenderTeleport).toBe(true)
-        expect(vm.shouldRenderPopperContent).toBe(true)
-        await nextTick()
-        expect(onShow).toHaveBeenCalled()
-        expect(vm.shouldRenderTeleport).toBe(true)
-        expect(vm.shouldRenderPopperContent).toBe(true)
-
-        open.value = false
-        expect(vm.shouldRenderTeleport).toBe(true)
-        expect(vm.shouldRenderPopperContent).toBe(true)
-        await nextTick()
-        expect(vm.leaving).toBe(true)
-        expect(vm.shouldRenderTeleport).toBe(true)
-        expect(vm.shouldRenderPopperContent).toBe(false)
-        await nextTick()
-        expect(vm.leaving).toBe(true)
-        // manually calling onTransitionLeave, because we stubbed Transition component.
-        vm.onTransitionLeave()
-        await nextTick()
-        expect(vm.shouldRenderTeleport).toBe(false)
-        expect(vm.shouldRenderPopperContent).toBe(false)
-        await nextTick()
-
-        /**
-         * NOTE for commenting this line
-         * Since vm.leaving = false is dispatched by `<transition /> after leave event`
-         * In our test transition is stubbed so that we cannot assert on this value for validate it
-         * It should be able to set vm.leaving to false because when the transition ends this event
-         * will be triggered.
-         */
-
-        // expect(vm.leaving).toBe(false)
-
-        expect(vm.shouldRenderTeleport).toBe(false)
-        expect(vm.shouldRenderPopperContent).toBe(false)
-        expect(onHide).toHaveBeenCalled()
-      })
-
-      it('it should be able to interrupt showing', async () => {
-        wrapper = createComponent()
-        await nextTick()
-
-        const { vm } = wrapper
-        expect(vm.shouldRenderTeleport).toBe(false)
-        expect(vm.shouldRenderPopperContent).toBe(false)
-
-        open.value = true
-        await nextTick()
-        expect(vm.shouldRenderTeleport).toBe(true)
-        expect(vm.shouldRenderPopperContent).toBe(false)
+    describe('content rendering', () => {
+      it('should not show the content when disabled', async () => {
+        wrapper = createComponent({
+          disabled: true,
+        })
 
         await nextTick()
-        expect(vm.shouldRenderTeleport).toBe(true)
-        expect(vm.shouldRenderPopperContent).toBe(true)
-        expect(onShow).not.toHaveBeenCalled()
-        open.value = false
-        await nextTick()
-        expect(onShow).not.toHaveBeenCalled()
-        expect(vm.shouldRenderTeleport).toBe(true)
-        expect(vm.shouldRenderPopperContent).toBe(true)
-        await nextTick()
-        // manually calling onTransitionLeave, because we stubbed Transition component.
-        vm.onTransitionLeave()
-        await nextTick()
-        expect(vm.shouldRenderTeleport).toBe(false)
-        expect(vm.shouldRenderPopperContent).toBe(false)
-      })
 
-      it('should be able to interrupt hiding', async () => {
-        wrapper = createComponent()
-        const { vm } = wrapper
-        await nextTick()
-
-        open.value = true
-        await nextTick()
-        await nextTick()
-        await nextTick()
-        expect(vm.shouldRenderTeleport).toBe(true)
-        expect(vm.shouldRenderPopperContent).toBe(true)
-        expect(onShow).toHaveBeenCalled()
-        expect(onHide).not.toHaveBeenCalled()
-
-        open.value = false
-        await nextTick()
-        expect(vm.leaving).toBe(true)
-        await nextTick()
-        expect(vm.shouldRenderTeleport).toBe(true)
-        expect(vm.shouldRenderPopperContent).toBe(false)
-        expect(onHide).not.toHaveBeenCalled()
-
-        open.value = true
-        await nextTick()
-        expect(vm.leaving).toBe(false)
-        expect(vm.entering).toBe(true)
+        expect(wrapper.vm.shouldShow).toBe(false)
       })
     })
 
@@ -245,7 +132,6 @@ describe('<ElTooltipContent />', () => {
 
       it('should be able to enter trigger', async () => {
         const { vm } = wrapper
-        expect(vm.shouldShowPopperContent).toBe(true)
         expect(onOpen).not.toHaveBeenCalled()
         const enterEvent = new MouseEvent('mouseenter')
         vm.onContentEnter(enterEvent)
@@ -272,7 +158,6 @@ describe('<ElTooltipContent />', () => {
         await nextTick()
         const { vm } = wrapper
 
-        expect(vm.shouldShowPopperContent).toBe(true)
         expect(onOpen).not.toHaveBeenCalled()
         const enterEvent = new MouseEvent('mouseenter')
         vm.onContentEnter(enterEvent)
@@ -282,6 +167,40 @@ describe('<ElTooltipContent />', () => {
         vm.onContentLeave(leaveEvent)
         expect(onClose).not.toHaveBeenCalled()
       })
+
+      describe('onCloseOutside', () => {
+        beforeEach(() => {
+          // Have to mock this ref because we are not going to render the content in this component
+          wrapper.vm.contentRef = {
+            popperContentRef: document.createElement('div'),
+          } as any
+        })
+
+        it('should not close the content after click outside when trigger is hover', async () => {
+          document.body.click()
+          await nextTick()
+          expect(onClose).not.toHaveBeenCalled()
+        })
+
+        it('should not close the content after click outside when controlled', async () => {
+          controlled.value = true
+          trigger.value = 'click'
+          await nextTick()
+
+          document.body.click()
+          await nextTick()
+          expect(onClose).not.toHaveBeenCalled()
+        })
+
+        it('should close component after click outside', async () => {
+          trigger.value = 'click'
+
+          document.body.click()
+          await nextTick()
+
+          expect(onClose).toHaveBeenCalled()
+        })
+      })
     })
   })
 })

packages/components/tooltip/__tests__/tooltip.spec.ts

@@ -99,12 +99,12 @@ describe('<ElTooltip />', () => {
 
       const trigger$ = findTrigger()
       const triggerEl = trigger$.find('.el-tooltip__trigger')
-      await triggerEl.trigger('mousedown')
+      await triggerEl.trigger('click')
       jest.runAllTimers()
       await rAF()
       expect(wrapper.emitted()).toHaveProperty('show')
 
-      await triggerEl.trigger('mousedown')
+      await triggerEl.trigger('click')
       jest.runAllTimers()
       await rAF()
       expect(wrapper.emitted()).toHaveProperty('hide')

packages/components/tooltip/__tests__/trigger.spec.ts

@@ -69,7 +69,7 @@ describe('<ElTooltipTrigger />', () => {
         await nextTick()
         expect(onOpen).not.toHaveBeenCalled()
         const mousedownEvt = new MouseEvent('mousedown')
-        vm.onMousedown(mousedownEvt)
+        vm.onClick(mousedownEvt)
         await nextTick()
         expect(onToggle).not.toHaveBeenCalled()
         const mouseenterEvt = new MouseEvent('mouseenter')
@@ -115,7 +115,7 @@ describe('<ElTooltipTrigger />', () => {
           trigger: 'click',
         })
         const mousedownEvt = new MouseEvent('mousedown')
-        vm.onMousedown(mousedownEvt)
+        vm.onClick(mousedownEvt)
         await nextTick()
         await wrapper.setProps({
           trigger: 'hover',

packages/components/tooltip/src/content.vue

@@ -1,13 +1,14 @@
 <template>
-  <el-teleport
-    v-if="shouldRenderTeleport"
-    :disabled="!teleported"
-    :container="POPPER_CONTAINER_SELECTOR"
-  >
-    <transition :name="transition" @after-leave="onTransitionLeave">
+  <teleport :disabled="!teleported" :to="POPPER_CONTAINER_SELECTOR">
+    <transition
+      :name="transition"
+      @after-leave="onTransitionLeave"
+      @before-enter="onBeforeEnter"
+      @after-enter="onAfterShow"
+    >
       <el-popper-content
-        v-if="shouldRenderPopperContent"
-        v-show="shouldShowPopperContent"
+        v-if="shouldRender"
+        v-show="shouldShow"
         ref="contentRef"
         v-bind="$attrs"
         :aria-hidden="ariaHidden"
@@ -34,19 +35,18 @@
         </el-visually-hidden>
       </el-popper-content>
     </transition>
-  </el-teleport>
+  </teleport>
 </template>
 
 <script lang="ts">
-import { computed, defineComponent, inject, nextTick, ref, unref } from 'vue'
+import { computed, defineComponent, inject, ref, unref, watch } from 'vue'
+import { onClickOutside } from '@vueuse/core'
 import { ElPopperContent } from '@element-plus/components/popper'
 import { ElVisuallyHidden } from '@element-plus/components/visual-hidden'
-import { ElTeleport } from '@element-plus/components/teleport'
 import { composeEventHandlers } from '@element-plus/utils/dom'
 import {
   useEscapeKeydown,
   POPPER_CONTAINER_SELECTOR,
-  useDelayedRender,
 } from '@element-plus/hooks'
 
 import { useTooltipContentProps } from './tooltip'
@@ -55,7 +55,6 @@ import { TOOLTIP_INJECTION_KEY } from './tokens'
 export default defineComponent({
   name: 'ElTooltipContent',
   components: {
-    ElTeleport,
     ElPopperContent,
     ElVisuallyHidden,
   },
@@ -77,76 +76,22 @@ export default defineComponent({
       return props.persistent
     })
 
+    const shouldRender = computed(() => {
+      return unref(persistentRef) ? true : unref(open)
+    })
+
+    const shouldShow = computed(() => {
+      return props.disabled ? false : unref(open)
+    })
+
     const contentStyle = computed(() => (props.style ?? {}) as any)
-    const shouldRenderTeleport = computed(() => {
-      if (unref(persistentRef)) return true
-      return unref(unref(entering) ? open : intermediateOpen)
-    })
 
-    const shouldRenderPopperContent = computed(() => {
-      if (unref(persistentRef)) return true
-      return unref(unref(leaving) ? open : intermediateOpen)
-    })
-
-    const shouldShowPopperContent = computed(() => {
-      // This is for control persistent mode transition
-      // When persistent this element will always be rendered, we simply use v-show to control the transition
-      if (unref(persistentRef)) {
-        return unref(unref(leaving) ? open : intermediateOpen)
-      }
-      return true
-    })
-
-    const ariaHidden = computed(
-      () =>
-        !(unref(shouldRenderPopperContent) && unref(shouldShowPopperContent))
-    )
+    const ariaHidden = computed(() => !unref(open))
 
     useEscapeKeydown(onClose)
 
-    useDelayedRender({
-      indicator: open,
-      intermediateIndicator: intermediateOpen,
-      shouldSetIntermediate: (step) => {
-        // we don't want to set the intermediateOpen because we want the transition to finish.
-        // After transition finishes, with the hook after-leave we can call intermediate.value = false
-        return step === 'hide' ? false : true
-      },
-      beforeShow: () => {
-        // indicates interruption of hide transition
-        if (unref(leaving)) {
-          leaving.value = false
-          intermediateOpen.value = false
-        }
-        entering.value = true
-      },
-      beforeHide: () => {
-        // indicates interruption of show transition
-        if (unref(entering)) {
-          entering.value = false
-          return
-        }
-        leaving.value = true
-      },
-      afterShow: () => {
-        if (!unref(open)) return
-        entering.value = false
-        onShow()
-        nextTick(() => {
-          unref(contentRef)?.updatePopper()
-        })
-      },
-      afterHide: () => {
-        if (unref(open)) return
-        // prevent the content from hiding if it's still open
-        onHide()
-      },
-    })
-
     const onTransitionLeave = () => {
-      if (unref(open)) return
-      leaving.value = false
-      intermediateOpen.value = false
+      onHide()
     }
 
     const stopWhenControlled = () => {
@@ -154,7 +99,7 @@ export default defineComponent({
     }
 
     const onContentEnter = composeEventHandlers(stopWhenControlled, () => {
-      if (props.enterable) {
+      if (props.enterable && unref(trigger) === 'hover') {
         onOpen()
       }
     })
@@ -165,6 +110,41 @@ export default defineComponent({
       }
     })
 
+    const onBeforeEnter = () => {
+      contentRef.value?.updatePopper?.()
+    }
+
+    const onAfterShow = () => {
+      onShow()
+    }
+
+    let stopHandle: ReturnType<typeof onClickOutside>
+
+    watch(
+      () => unref(open),
+      (val) => {
+        if (val) {
+          stopHandle = onClickOutside(
+            computed(() => {
+              return contentRef.value?.popperContentRef
+            }),
+            () => {
+              if (unref(controlled)) return
+              const $trigger = unref(trigger)
+              if ($trigger !== 'hover') {
+                onClose()
+              }
+            }
+          )
+        } else {
+          stopHandle?.()
+        }
+      },
+      {
+        flush: 'post',
+      }
+    )
+
     return {
       ariaHidden,
       entering,
@@ -173,11 +153,12 @@ export default defineComponent({
       intermediateOpen,
       contentStyle,
       contentRef,
-      shouldRenderTeleport,
-      shouldRenderPopperContent,
-      shouldShowPopperContent,
+      shouldRender,
+      shouldShow,
       open,
       POPPER_CONTAINER_SELECTOR,
+      onAfterShow,
+      onBeforeEnter,
       onContentEnter,
       onContentLeave,
       onTransitionLeave,

packages/components/tooltip/src/tooltip.ts

@@ -41,6 +41,9 @@ export const useTooltipContentProps = {
       type: Boolean,
       default: true,
     },
+    disabled: {
+      type: Boolean,
+    },
   }),
 }
 

packages/components/tooltip/src/tooltip.vue

@@ -12,6 +12,7 @@
       :aria-label="ariaLabel"
       :boundaries-padding="boundariesPadding"
       :content="content"
+      :disabled="disabled"
       :effect="effect"
       :enterable="enterable"
       :fallback-placements="fallbackPlacements"

packages/components/tooltip/src/trigger.vue

@@ -6,11 +6,11 @@
     :virtual-triggering="virtualTriggering"
     class="el-tooltip__trigger"
     @blur="onBlur"
+    @click="onClick"
     @contextmenu="onContextMenu"
     @focus="onFocus"
     @mouseenter="onMouseenter"
     @mouseleave="onMouseleave"
-    @mousedown="onMousedown"
     @keydown="onKeydown"
   >
     <slot />
@@ -55,7 +55,7 @@ export default defineComponent({
       stopWhenControlledOrDisabled,
       whenTrigger(trigger, 'hover', onClose)
     )
-    const onMousedown = composeEventHandlers(
+    const onClick = composeEventHandlers(
       stopWhenControlledOrDisabled,
       whenTrigger(trigger, 'click', (e) => {
         // distinguish left click
@@ -99,7 +99,7 @@ export default defineComponent({
       onFocus,
       onMouseenter,
       onMouseleave,
-      onMousedown,
+      onClick,
       onKeydown,
       open,
       id,