ronantakizawa/codereview-bench huggingface dataset:
Repository: element-plus/element-plus
File: docs/examples/drawer/resizable-drawer.vue (Vue)

Code before:
<template>
  <el-button plain @click="visible = true">Open a resizable Drawer</el-button>
  <el-drawer v-model="visible" resizable>
    <template #header="{ titleId, titleClass }">
      <h4 :id="titleId" :class="titleClass">This is a custom header!</h4>
    </template>
    This is drawer content.
  </el-drawer>
</template>

<script lang="ts" setup>
import { ref } from 'vue'

const visible = ref(false)
</script>


Review comment (suggestion):
It looks like ```cursor``` will be obscured by `mask`.




https://github.com/user-attachments/assets/39f27822-20b6-4af6-9e80-d025eb4f1973




Code after:
<template>
  <el-segmented
    v-model="direction"
    :options="options"
    size="large"
    @change="visible = true"
  />

  <el-drawer v-model="visible" :direction="direction" resizable>
    <template #header="{ titleId, titleClass }">
      <h4 :id="titleId" :class="titleClass">This is a custom header!</h4>
    </template>
    This is drawer content.
  </el-drawer>
</template>

<script lang="ts" setup>
import { ref } from 'vue'

import type { DrawerProps } from 'element-plus'

const visible = ref(false)
const direction = ref<DrawerProps['direction']>()

const options = [
  { label: 'Top', value: 'ttb' },
  { label: 'Right', value: 'rtl' },
  { label: 'Bottom', value: 'btt' },
Repository: element-plus/element-plus
File: docs/examples/drawer/resizable-drawer.vue (Vue)

Code before:
<template>
  <el-radio-group v-model="direction">
    <el-radio value="ltr">left to right</el-radio>
    <el-radio value="rtl">right to left</el-radio>
    <el-radio value="ttb">top to bottom</el-radio>
    <el-radio value="btt">bottom to top</el-radio>
  </el-radio-group>

  <el-button type="primary" style="margin-left: 16px" @click="visible = true">
    Open a resizable Drawer
  </el-button>

  <el-drawer v-model="visible" :direction="direction" resizable>
    <template #header="{ titleId, titleClass }">
      <h4 :id="titleId" :class="titleClass">This is a custom header!</h4>
    </template>
    This is drawer content.
  </el-drawer>
</template>

<script lang="ts" setup>
import { ref } from 'vue'

import type { DrawerProps } from 'element-plus'

const visible = ref(false)
const direction = ref<DrawerProps['direction']>('rtl')
</script>

Review comment (suggestion):
I think it would be more convenient to bind it directly to Button.Group or Segmented, like naive, so that the drawer can be opened directly.

But don't change it yet, as other members may not agree with it. Let's see what everyone thinks.

Code after:
<template>
  <el-segmented
    v-model="direction"
    :options="options"
    size="large"
    @change="visible = true"
  />

  <el-drawer v-model="visible" :direction="direction" resizable>
    <template #header="{ titleId, titleClass }">
      <h4 :id="titleId" :class="titleClass">This is a custom header!</h4>
    </template>
    This is drawer content.
  </el-drawer>
</template>

<script lang="ts" setup>
import { ref } from 'vue'

import type { DrawerProps } from 'element-plus'

const visible = ref(false)
const direction = ref<DrawerProps['direction']>()

const options = [
  { label: 'Top', value: 'ttb' },
  { label: 'Right', value: 'rtl' },
  { label: 'Bottom', value: 'btt' },
Repository: element-plus/element-plus
File: packages/components/time-picker/src/common/picker-range-trigger.vue (Vue)

Code before:
<template>
  <div
    ref="wrapperRef"
    :class="[nsDate.is('active', isFocused), $attrs.class]"
    :style="($attrs.style as CSSProperties)"
    @click="handleClick"
    @mouseenter="handleMouseEnter"
    @mouseleave="handleMouseLeave"
    @touchstart.passive="handleTouchStart"
  >
    <slot name="prefix" />
    <input
      v-bind="attrs"
      :id="(id && id[0]) ?? inputId"
      ref="inputRef"
      :name="name && name[0]"
      :placeholder="startPlaceholder"
      :value="modelValue && modelValue[0]"
      :class="nsRange.b('input')"
      :disabled="disabled"
      @input="handleStartInput"
      @change="handleStartChange"
    />
    <slot name="range-separator" />
    <input
      v-bind="attrs"
      :id="id && id[1]"
      ref="endInputRef"
      :name="name && name[1]"
      :placeholder="endPlaceholder"
      :value="modelValue && modelValue[1]"
      :class="nsRange.b('input')"
      :disabled="disabled"
      @input="handleEndInput"
      @change="handleEndChange"
    />
    <slot name="suffix" />
  </div>
</template>

Review comment (suggestion):
```suggestion
      :id="inputId"
```

You can use `inputId` directly here, as useFormItemInputId has already [handled](https://github.com/element-plus/element-plus/blob/66a0ca27587f2404eb7dab86bceb1804ca2b96bc/packages/components/form/src/hooks/use-form-item.ts#L84) `props.id`.

Code after:
<template>
  <div
    ref="wrapperRef"
    :class="[nsDate.is('active', isFocused), $attrs.class]"
    :style="($attrs.style as CSSProperties)"
    @click="handleClick"
    @mouseenter="handleMouseEnter"
    @mouseleave="handleMouseLeave"
    @touchstart.passive="handleTouchStart"
  >
    <slot name="prefix" />
    <input
      v-bind="attrs"
      :id="inputId"
      ref="inputRef"
      :name="name && name[0]"
      :placeholder="startPlaceholder"
      :value="modelValue && modelValue[0]"
      :class="nsRange.b('input')"
      :disabled="disabled"
      @input="handleStartInput"
      @change="handleStartChange"
    />
    <slot name="range-separator" />
    <input
      v-bind="attrs"
      :id="id && id[1]"
      ref="endInputRef"
      :name="name && name[1]"
      :placeholder="endPlaceholder"
      :value="modelValue && modelValue[1]"
      :class="nsRange.b('input')"
      :disabled="disabled"
      @input="handleEndInput"
      @change="handleEndChange"
    />
    <slot name="suffix" />
  </div>
</template>
Repository: element-plus/element-plus
File: packages/components/time-picker/src/common/picker-range-trigger.vue (Vue)

Code before:
import { useFormItem, useFormItemInputId } from '@element-plus/components/form'

import type { CSSProperties } from 'vue'

defineOptions({
  name: 'PickerRangeTrigger',
  inheritAttrs: false,
})

const props = defineProps(timePickerRangeTriggerProps)
const emit = defineEmits([
  'mouseenter',
  'mouseleave',
  'click',
  'touchstart',
  'focus',
  'blur',
  'startInput',
  'endInput',
  'startChange',
  'endChange',
])

const { formItem: elFormItem } = useFormItem()
const { inputId } = useFormItemInputId(
  reactive({ ...props, id: computed(() => props.id?.[0]) }),
  {
    formItemContext: elFormItem,
  }
)

const attrs = useAttrs()
const nsDate = useNamespace('date')
const nsRange = useNamespace('range')

const inputRef = ref<HTMLInputElement>()
const endInputRef = ref<HTMLInputElement>()

const { wrapperRef, isFocused } = useFocusController(inputRef, {
  disabled: computed(() => props.disabled),
})

const handleClick = (evt: MouseEvent) => {
  emit('click', evt)
}

const handleMouseEnter = (evt: MouseEvent) => {
  emit('mouseenter', evt)
}

const handleMouseLeave = (evt: MouseEvent) => {

Review comment (suggestion):
**nit**:

```suggestion
  reactive({ id: computed(() => props.id?.[0]) }),
```

Code after:
import { useFormItem, useFormItemInputId } from '@element-plus/components/form'

import type { CSSProperties } from 'vue'

defineOptions({
  name: 'PickerRangeTrigger',
  inheritAttrs: false,
})

const props = defineProps(timePickerRangeTriggerProps)
const emit = defineEmits([
  'mouseenter',
  'mouseleave',
  'click',
  'touchstart',
  'focus',
  'blur',
  'startInput',
  'endInput',
  'startChange',
  'endChange',
])

const { formItem } = useFormItem()
const { inputId } = useFormItemInputId(
  reactive({ id: computed(() => props.id?.[0]) }),
  {
    formItemContext: formItem,
  }
)

const attrs = useAttrs()
const nsDate = useNamespace('date')
const nsRange = useNamespace('range')

const inputRef = ref<HTMLInputElement>()
const endInputRef = ref<HTMLInputElement>()

const { wrapperRef, isFocused } = useFocusController(inputRef, {
  disabled: computed(() => props.disabled),
})

const handleClick = (evt: MouseEvent) => {
  emit('click', evt)
}

const handleMouseEnter = (evt: MouseEvent) => {
  emit('mouseenter', evt)
}

const handleMouseLeave = (evt: MouseEvent) => {
Repository: element-plus/element-plus
File: packages/components/time-picker/src/common/picker-range-trigger.vue (Vue)

Code before:
import { useAttrs, useFocusController, useNamespace } from '@element-plus/hooks'
import { timePickerRangeTriggerProps } from './props'
import { useFormItem, useFormItemInputId } from '@element-plus/components/form'

import type { CSSProperties } from 'vue'

defineOptions({
  name: 'PickerRangeTrigger',
  inheritAttrs: false,
})

const props = defineProps(timePickerRangeTriggerProps)
const emit = defineEmits([
  'mouseenter',
  'mouseleave',
  'click',
  'touchstart',
  'focus',
  'blur',
  'startInput',
  'endInput',
  'startChange',
  'endChange',
])

const { formItem: elFormItem } = useFormItem()
const { inputId } = useFormItemInputId(
  reactive({ ...props, id: computed(() => props.id?.[0]) }),
  {
    formItemContext: elFormItem,
  }
)

const attrs = useAttrs()
const nsDate = useNamespace('date')
const nsRange = useNamespace('range')

const inputRef = ref<HTMLInputElement>()
const endInputRef = ref<HTMLInputElement>()

const { wrapperRef, isFocused } = useFocusController(inputRef, {
  disabled: computed(() => props.disabled),
})

const handleClick = (evt: MouseEvent) => {
  emit('click', evt)
}

const handleMouseEnter = (evt: MouseEvent) => {
  emit('mouseenter', evt)
}

Review comment (suggestion):
```suggestion
const { formItem } = useFormItem()
```

Code after:
import { useAttrs, useFocusController, useNamespace } from '@element-plus/hooks'
import { timePickerRangeTriggerProps } from './props'
import { useFormItem, useFormItemInputId } from '@element-plus/components/form'

import type { CSSProperties } from 'vue'

defineOptions({
  name: 'PickerRangeTrigger',
  inheritAttrs: false,
})

const props = defineProps(timePickerRangeTriggerProps)
const emit = defineEmits([
  'mouseenter',
  'mouseleave',
  'click',
  'touchstart',
  'focus',
  'blur',
  'startInput',
  'endInput',
  'startChange',
  'endChange',
])

const { formItem } = useFormItem()
const { inputId } = useFormItemInputId(
  reactive({ id: computed(() => props.id?.[0]) }),
  {
    formItemContext: formItem,
  }
)

const attrs = useAttrs()
const nsDate = useNamespace('date')
const nsRange = useNamespace('range')

const inputRef = ref<HTMLInputElement>()
const endInputRef = ref<HTMLInputElement>()

const { wrapperRef, isFocused } = useFocusController(inputRef, {
  disabled: computed(() => props.disabled),
})

const handleClick = (evt: MouseEvent) => {
  emit('click', evt)
}

const handleMouseEnter = (evt: MouseEvent) => {
  emit('mouseenter', evt)
}
Repository: element-plus/element-plus
File: packages/components/cascader-panel/src/node.ts (TypeScript)

Code before:
import { isArray, isEmpty, isFunction, isUndefined } from '@element-plus/utils'

import type {
  CascaderConfig,
  CascaderNodePathValue,
  CascaderNodeValue,
  CascaderOption,
} from './types'

export type * from './types'

type ChildrenData = CascaderOption[] | undefined

let uid = 0

const calculatePathNodes = (node: Node) => {
  const nodes = [node]
  let { parent } = node

  while (parent) {
    nodes.unshift(parent)
    parent = parent.parent
  }

  return nodes
}

class Node {
  readonly uid: number = uid++
  readonly level: number
  readonly value: CascaderNodeValue
  readonly label: string
  readonly pathNodes: Node[]
  readonly pathValues: CascaderNodePathValue
  readonly pathLabels: string[]

Review comment (refactor):
Now we should be able to remove this line. And imports types from `types.ts`.

Code after:
import { isArray, isEmpty, isFunction, isUndefined } from '@element-plus/utils'

import type {
  CascaderConfig,
  CascaderNodePathValue,
  CascaderNodeValue,
  CascaderOption,
} from './types'

type ChildrenData = CascaderOption[] | undefined

let uid = 0

const calculatePathNodes = (node: Node) => {
  const nodes = [node]
  let { parent } = node

  while (parent) {
    nodes.unshift(parent)
    parent = parent.parent
  }

  return nodes
}

class Node {
  readonly uid: number = uid++
  readonly level: number
  readonly value: CascaderNodeValue
  readonly label: string
  readonly pathNodes: Node[]
  readonly pathValues: CascaderNodePathValue
  readonly pathLabels: string[]

  childrenData: ChildrenData
Repository: element-plus/element-plus
File: packages/components/tooltip/src/trigger.vue (Vue)

Code before:
const props = defineProps(useTooltipTriggerProps)

const ns = useNamespace('tooltip')
const { controlled, id, open, onOpen, onClose, onToggle } = inject(
  TOOLTIP_INJECTION_KEY,
  undefined
)!

const triggerRef = ref<OnlyChildExpose | null>(null)

const stopWhenControlledOrDisabled = () => {
  if (unref(controlled) || props.disabled) {
    return true
  }
}
const trigger = toRef(props, 'trigger')
const onMouseenter = composeEventHandlers(
  stopWhenControlledOrDisabled,
  whenTrigger(trigger, 'hover', (e) => {
    onOpen(e)

    if (props.focusOnTarget && e.target) {
      nextTick(() => {
        focusElement(e.target as HTMLElement, {
          preventScroll: true,
          focusVisible: false,
        })
      })
    }
  })
)
const onMouseleave = composeEventHandlers(
  stopWhenControlledOrDisabled,
  whenTrigger(trigger, 'hover', onClose)
)
const onClick = composeEventHandlers(
  stopWhenControlledOrDisabled,
  whenTrigger(trigger, 'click', (e) => {
    // distinguish left click
    if ((e as MouseEvent).button === 0) {
      onToggle(e)
    }
  })
)

const onFocus = composeEventHandlers(
  stopWhenControlledOrDisabled,
  whenTrigger(trigger, 'focus', onOpen)
)

const onBlur = composeEventHandlers(

Review comment (refactor):
<img width="1053" height="299" alt="Image" src="https://github.com/user-attachments/assets/7b293aa1-33d7-4a7f-9695-968e363974c6" />

It seems that most browsers do not support the `focusVisible` option. Can we remove this option? Will this have any impact?

Code after:
const props = defineProps(useTooltipTriggerProps)

const ns = useNamespace('tooltip')
const { controlled, id, open, onOpen, onClose, onToggle } = inject(
  TOOLTIP_INJECTION_KEY,
  undefined
)!

const triggerRef = ref<OnlyChildExpose | null>(null)

const stopWhenControlledOrDisabled = () => {
  if (unref(controlled) || props.disabled) {
    return true
  }
}
const trigger = toRef(props, 'trigger')
const onMouseenter = composeEventHandlers(
  stopWhenControlledOrDisabled,
  whenTrigger(trigger, 'hover', (e) => {
    onOpen(e)

    if (props.focusOnTarget && e.target) {
      nextTick(() => {
        focusElement(e.target as HTMLElement, { preventScroll: true })
      })
    }
  })
)
const onMouseleave = composeEventHandlers(
  stopWhenControlledOrDisabled,
  whenTrigger(trigger, 'hover', onClose)
)
const onClick = composeEventHandlers(
  stopWhenControlledOrDisabled,
  whenTrigger(trigger, 'click', (e) => {
    // distinguish left click
    if ((e as MouseEvent).button === 0) {
      onToggle(e)
    }
  })
)

const onFocus = composeEventHandlers(
  stopWhenControlledOrDisabled,
  whenTrigger(trigger, 'focus', onOpen)
)

const onBlur = composeEventHandlers(
  stopWhenControlledOrDisabled,
  whenTrigger(trigger, 'focus', onClose)
)
Repository: element-plus/element-plus
File: packages/utils/dom/aria.ts (TypeScript)

Code before:
    eventName = 'KeyboardEvent'
  } else {
    eventName = 'HTMLEvents'
  }
  const evt = document.createEvent(eventName)

  evt.initEvent(name, ...opts)
  elm.dispatchEvent(evt)
  return elm
}

export const isLeaf = (el: HTMLElement) => !el.getAttribute('aria-owns')

export const getSibling = (
  el: HTMLElement,
  distance: number,
  elClass: string
) => {
  const { parentNode } = el
  if (!parentNode) return null
  const siblings = parentNode.querySelectorAll(elClass)
  const index = Array.prototype.indexOf.call(siblings, el)
  return siblings[index + distance] || null
}

export const focusNode = (el: HTMLElement) => {
  if (!el) return
  el.focus()
  !isLeaf(el) && el.click()
}


Review comment (suggestion):
```suggestion
  if (cleanup) {
```
When `cleanup` is `true`, `isElement(el)` must be `true`.

Code after:
  el: HTMLElement,
  distance: number,
  elClass: string
) => {
  const { parentNode } = el
  if (!parentNode) return null
  const siblings = parentNode.querySelectorAll(elClass)
  const index = Array.prototype.indexOf.call(siblings, el)
  return siblings[index + distance] || null
}

export const focusElement = (
  el?: HTMLElement | { focus: () => void } | null,
  options?: FocusOptions
) => {
  if (!el || !el.focus) return
  let cleanup: boolean = false

  if (isElement(el) && !isFocusable(el) && !el.getAttribute('tabindex')) {
    el.setAttribute('tabindex', '-1')
    cleanup = true
  }

  el.focus(options)

  if (isElement(el) && cleanup) {
    el.removeAttribute('tabindex')
  }
}

export const focusNode = (el: HTMLElement) => {
  if (!el) return
  focusElement(el)
  !isLeaf(el) && el.click()
}
Repository: element-plus/element-plus
File: packages/components/image-viewer/src/image-viewer.vue (Vue)

Code before:
}

function reset() {
  transform.value = {
    scale: 1,
    deg: 0,
    offsetX: 0,
    offsetY: 0,
    enableTransition: false,
  }
}

function toggleMode() {
  if (loading.value || loadError.value) return

  const modeNames = keysOf(modes)
  const modeValues = Object.values(modes)
  const currentMode = mode.value.name
  const index = modeValues.findIndex((i) => i.name === currentMode)
  const nextIndex = (index + 1) % modeNames.length
  mode.value = modes[modeNames[nextIndex]]
  reset()
}

function setActiveItem(index: number) {
  loadError.value &&= false
  const len = props.urlList.length
  activeIndex.value = (index + len) % len
}

function prev() {
  if (isFirst.value && !props.infinite) return
  setActiveItem(activeIndex.value - 1)
}

function next() {
  if (isLast.value && !props.infinite) return
  setActiveItem(activeIndex.value + 1)
}

function handleActions(action: ImageViewerAction, options = {}) {
  if (loading.value || loadError.value) return
  const { minScale, maxScale } = props
  const { zoomRate, rotateDeg, enableTransition } = {
    zoomRate: props.zoomRate,
    rotateDeg: 90,
    enableTransition: true,
    ...options,
  }
  switch (action) {
    case 'zoomOut':

Review comment (suggestion):
I think this can be simplified.

```suggestion
  loadError.value = false
```

Code after:
  useEventListener(document, 'mouseup', () => {
    removeMousemove()
  })

  e.preventDefault()
}

function reset() {
  transform.value = {
    scale: 1,
    deg: 0,
    offsetX: 0,
    offsetY: 0,
    enableTransition: false,
  }
}

function toggleMode() {
  if (loading.value || loadError.value) return

  const modeNames = keysOf(modes)
  const modeValues = Object.values(modes)
  const currentMode = mode.value.name
  const index = modeValues.findIndex((i) => i.name === currentMode)
  const nextIndex = (index + 1) % modeNames.length
  mode.value = modes[modeNames[nextIndex]]
  reset()
}

function setActiveItem(index: number) {
  loadError.value = false
  const len = props.urlList.length
  activeIndex.value = (index + len) % len
}

function prev() {
  if (isFirst.value && !props.infinite) return
  setActiveItem(activeIndex.value - 1)
}

function next() {
  if (isLast.value && !props.infinite) return
  setActiveItem(activeIndex.value + 1)
}

function handleActions(action: ImageViewerAction, options = {}) {
  if (loading.value || loadError.value) return
  const { minScale, maxScale } = props
  const { zoomRate, rotateDeg, enableTransition } = {
    zoomRate: props.zoomRate,
    rotateDeg: 90,
Repository: element-plus/element-plus
File: packages/components/image-viewer/src/image-viewer.vue (Vue)

Code before:
                <el-icon @click="handleActions('clockwise')">
                  <RefreshRight />
                </el-icon>
              </slot>
            </div>
          </div>
          <!-- CANVAS -->
          <div :class="ns.e('canvas')">
            <slot v-if="loadError" name="viewer-error">
              <div :class="ns.e('error')">{{ t('el.image.error') }}</div>
            </slot>
            <img
              v-else
              ref="imgRef"
              :src="currentImg"
              :style="imgStyle"
              :class="ns.e('img')"
              :crossorigin="crossorigin"
              @load="handleImgLoad"
              @error="handleImgError"
              @mousedown="handleMouseDown"
            />
          </div>
          <slot />
        </el-focus-trap>
      </div>
    </transition>
  </el-teleport>
</template>

<script lang="ts" setup>
import {
  computed,
  effectScope,
  markRaw,
  nextTick,
  onMounted,
  ref,
  shallowRef,
  watch,
} from 'vue'
import { useEventListener } from '@vueuse/core'
import { throttle } from 'lodash-unified'
import { useLocale, useNamespace, useZIndex } from '@element-plus/hooks'
import { EVENT_CODE } from '@element-plus/constants'
import { keysOf } from '@element-plus/utils'
import ElFocusTrap from '@element-plus/components/focus-trap'
import ElTeleport from '@element-plus/components/teleport'
import ElIcon from '@element-plus/components/icon'
import {
  ArrowLeft,

Review comment (suggestion):
I think we can keep the original structure and pass the `url` and `index` to the slot `viewer-error`.

Code after:
                <el-icon @click="handleActions('clockwise')">
                  <RefreshRight />
                </el-icon>
              </slot>
            </div>
          </div>
          <!-- CANVAS -->
          <div :class="ns.e('canvas')">
            <slot
              v-if="loadError && $slots['viewer-error']"
              name="viewer-error"
              :active-index="activeIndex"
              :src="currentImg"
            />
            <img
              v-else
              ref="imgRef"
              :key="currentImg"
              :src="currentImg"
              :style="imgStyle"
              :class="ns.e('img')"
              :crossorigin="crossorigin"
              @load="handleImgLoad"
              @error="handleImgError"
              @mousedown="handleMouseDown"
            />
          </div>
          <slot />
        </el-focus-trap>
      </div>
    </transition>
  </el-teleport>
</template>

<script lang="ts" setup>
import {
  computed,
  effectScope,
  markRaw,
  nextTick,
  onMounted,
  ref,
  shallowRef,
  watch,
} from 'vue'
import { useEventListener } from '@vueuse/core'
import { throttle } from 'lodash-unified'
import { useLocale, useNamespace, useZIndex } from '@element-plus/hooks'
import { EVENT_CODE } from '@element-plus/constants'
import { keysOf } from '@element-plus/utils'
import ElFocusTrap from '@element-plus/components/focus-trap'