ronantakizawa/codereview-bench huggingface dataset:
Repository: element-plus/element-plus
File: packages/components/checkbox/src/checkbox-button.vue (Vue)

Code before:
<template>
  <label :class="labelKls">
    <input
      :id="inputId"
      v-model="model"
      :class="ns.be('button', 'original')"
      type="checkbox"
      :indeterminate="indeterminate"
      :name="name"
      :tabindex="tabindex"
      :disabled="isDisabled"
      v-bind="inputBindings"
      @change="handleChange"
      @focus="isFocused = true"
      @blur="isFocused = false"
      @click.stop
    />

    <span
      v-if="$slots.default || label"
      :class="ns.be('button', 'inner')"
      :style="isChecked ? activeStyle : undefined"
    >
      <slot>{{ label }}</slot>
    </span>
  </label>
</template>

<script lang="ts" setup>
import { computed, inject, useSlots } from 'vue'
import { useNamespace } from '@element-plus/hooks'
import { checkboxGroupContextKey } from './constants'
import { useCheckbox } from './composables'

Review comment (suggestion):
I don’t think we need this prop, since `checkbox-button` doesn’t currently have an `indeterminate` state.

[CheckboxButton Attributes](https://element-plus.org/en-US/component/checkbox.html#checkboxbutton-attributes)

Code after:
<template>
  <label :class="labelKls">
    <input
      v-model="model"
      :class="ns.be('button', 'original')"
      type="checkbox"
      :name="name"
      :tabindex="tabindex"
      :disabled="isDisabled"
      v-bind="inputBindings"
      @change="handleChange"
      @focus="isFocused = true"
      @blur="isFocused = false"
      @click.stop
    />

    <span
      v-if="$slots.default || label"
      :class="ns.be('button', 'inner')"
      :style="isChecked ? activeStyle : undefined"
    >
      <slot>{{ label }}</slot>
    </span>
  </label>
</template>

<script lang="ts" setup>
import { computed, inject, useSlots } from 'vue'
import { useNamespace } from '@element-plus/hooks'
import { checkboxGroupContextKey } from './constants'
import { useCheckbox } from './composables'
import { checkboxEmits, checkboxProps } from './checkbox'
Repository: element-plus/element-plus
File: packages/components/checkbox/src/checkbox-button.vue (Vue)

Code before:
<template>
  <label :class="labelKls">
    <input
      :id="inputId"
      v-model="model"
      :class="ns.be('button', 'original')"
      type="checkbox"
      :name="name"
      :tabindex="tabindex"
      :disabled="isDisabled"
      v-bind="inputBindings"
      @change="handleChange"
      @focus="isFocused = true"
      @blur="isFocused = false"
      @click.stop
    />

    <span
      v-if="$slots.default || label"
      :class="ns.be('button', 'inner')"
      :style="isChecked ? activeStyle : undefined"
    >
      <slot>{{ label }}</slot>
    </span>
  </label>
</template>

<script lang="ts" setup>
import { computed, inject, useSlots } from 'vue'

Review comment (suggestion):
I tried to get `el-form-item` working with `el-check-button` in order to get back the `id` but didn't find a way to do it.
Perhaps it was intended to leave this input without an `id`.

```suggestion
```

Code after:
<template>
  <label :class="labelKls">
    <input
      v-model="model"
      :class="ns.be('button', 'original')"
      type="checkbox"
      :name="name"
      :tabindex="tabindex"
      :disabled="isDisabled"
      v-bind="inputBindings"
      @change="handleChange"
      @focus="isFocused = true"
      @blur="isFocused = false"
      @click.stop
    />

    <span
      v-if="$slots.default || label"
      :class="ns.be('button', 'inner')"
      :style="isChecked ? activeStyle : undefined"
    >
      <slot>{{ label }}</slot>
    </span>
  </label>
</template>

<script lang="ts" setup>
import { computed, inject, useSlots } from 'vue'
import { useNamespace } from '@element-plus/hooks'
Repository: element-plus/element-plus
File: packages/components/autocomplete/src/autocomplete.vue (Vue)

Code before:

  if (offsetTop + scrollHeight > scrollTop + suggestion.clientHeight) {
    suggestion.scrollTop += scrollHeight
  }
  if (offsetTop < scrollTop) {
    suggestion.scrollTop -= scrollHeight
  }
  highlightedIndex.value = index
  inputRef.value!.ref!.setAttribute(
    'aria-activedescendant',
    `${listboxId.value}-item-${highlightedIndex.value}`
  )
}

const stopHandle = onClickOutside(listboxRef, () => {
  // Prevent closing if focus is inside popper content
  if (popperRef.value?.isFocusInsideContent()) return
  suggestionVisible.value && close()
})

onBeforeUnmount(() => {
  stopHandle?.()
})

onMounted(() => {
  const inputElement = inputRef.value!.ref!
  ;[
    { key: 'role', value: 'textbox' },
    { key: 'aria-autocomplete', value: 'list' },
    { key: 'aria-controls', value: 'id' },
    {
      key: 'aria-activedescendant',
      value: `${listboxId.value}-item-${highlightedIndex.value}`,
    },
  ].forEach(({ key, value }) => inputElement.setAttribute(key, value))
  // get readonly attr
  readonly = inputElement.hasAttribute('readonly')
})

defineExpose({
  /** @description the index of the currently highlighted item */
  highlightedIndex,
  /** @description autocomplete whether activated */
  activated,
  /** @description remote search loading status */
  loading,
  /** @description el-input component instance */
  inputRef,
  /** @description el-tooltip component instance */
  popperRef,
  /** @description fetch suggestions result */

Review comment (suggestion):
```suggestion
  const inputElement = inputRef.value?.ref
  if (!inputElement) return
```

Code after:

  if (offsetTop + scrollHeight > scrollTop + suggestion.clientHeight) {
    suggestion.scrollTop += scrollHeight
  }
  if (offsetTop < scrollTop) {
    suggestion.scrollTop -= scrollHeight
  }
  highlightedIndex.value = index
  inputRef.value?.ref?.setAttribute(
    'aria-activedescendant',
    `${listboxId.value}-item-${highlightedIndex.value}`
  )
}

const stopHandle = onClickOutside(listboxRef, () => {
  // Prevent closing if focus is inside popper content
  if (popperRef.value?.isFocusInsideContent()) return
  suggestionVisible.value && close()
})

onBeforeUnmount(() => {
  stopHandle?.()
})

onMounted(() => {
  const inputElement = inputRef.value?.ref
  if (!inputElement) return
  ;[
    { key: 'role', value: 'textbox' },
    { key: 'aria-autocomplete', value: 'list' },
    { key: 'aria-controls', value: 'id' },
    {
      key: 'aria-activedescendant',
      value: `${listboxId.value}-item-${highlightedIndex.value}`,
    },
  ].forEach(({ key, value }) => inputElement.setAttribute(key, value))
  // get readonly attr
  readonly = inputElement.hasAttribute('readonly')
})

defineExpose({
  /** @description the index of the currently highlighted item */
  highlightedIndex,
  /** @description autocomplete whether activated */
  activated,
  /** @description remote search loading status */
  loading,
  /** @description el-input component instance */
  inputRef,
  /** @description el-tooltip component instance */
  popperRef,
Repository: element-plus/element-plus
File: packages/components/virtual-list/src/hooks/use-wheel.ts (TypeScript)

Code before:
const useWheel = (
  { atEndEdge, atStartEdge, layout }: ListWheelState,
  onWheelDelta: ListWheelHandler
) => {
  let frameHandle: number
  let offset = 0

  const hasReachedEdge = (offset: number) => {
    const edgeReached =
      (offset < 0 && atStartEdge.value) || (offset > 0 && atEndEdge.value)

    return edgeReached
  }

  const onWheel = (e: WheelEvent) => {
    cAF(frameHandle)

    let newOffset = 0
    const { deltaX, deltaY } = e
    const absX = Math.abs(e.deltaX)
    const absY = Math.abs(e.deltaY)
    if (absX === absY) {
      newOffset = layout.value === HORIZONTAL ? deltaX : deltaY
    } else {
      newOffset = absX > absY ? deltaX : deltaY
    }

    if (hasReachedEdge(offset) && hasReachedEdge(offset + newOffset)) return

    offset += newOffset

    if (!isFirefox()) {
      e.preventDefault()
    }

    frameHandle = rAF(() => {
      onWheelDelta(offset)
      offset = 0
    })
  }

  return {
    hasReachedEdge,
    onWheel,
  }
}

export default useWheel


Review comment (suggestion):
I feel like we can only allow the vertical layout for tabs.

I don't see a scenario where we need horizontal scroll while using the wheel scroll.

https://github.com/element-plus/element-plus/blob/328523e2916a037693b62d0e568d81ccf2770db2/packages/components/tabs/src/tab-nav.tsx#L137-L139

```diff
-        layout: computed(() =>
-          isHorizontal.value ? 'horizontal' : 'vertical'
-        ),
+        layout: computed(() => 'vertical'
```

Code after:
const useWheel = (
  { atEndEdge, atStartEdge, layout }: ListWheelState,
  onWheelDelta: ListWheelHandler
) => {
  let frameHandle: number
  let offset = 0

  const hasReachedEdge = (offset: number) => {
    const edgeReached =
      (offset < 0 && atStartEdge.value) || (offset > 0 && atEndEdge.value)

    return edgeReached
  }

  const onWheel = (e: WheelEvent) => {
    cAF(frameHandle)

    let { deltaX, deltaY } = e
    // Special case for windows machine with shift key + wheel scrolling
    if (e.shiftKey && deltaY !== 0) {
      deltaX = deltaY
      deltaY = 0
    }

    const newOffset = layout.value === HORIZONTAL ? deltaX : deltaY

    if (hasReachedEdge(offset) && hasReachedEdge(offset + newOffset)) return

    offset += newOffset

    if (!isFirefox()) {
      e.preventDefault()
    }

    frameHandle = rAF(() => {
      onWheelDelta(offset)
      offset = 0
    })
  }

  return {
    hasReachedEdge,
    onWheel,
  }
}

export default useWheel
Repository: element-plus/element-plus
File: packages/components/tree-v2/src/tree-node.vue (Vue)

Code before:
        height: itemSize + 'px',
      }"
    >
      <el-icon
        v-if="icon"
        :class="[
          ns.is('leaf', !!node?.isLeaf),
          ns.is('hidden', hiddenExpandIcon),
          {
            expanded: !node?.isLeaf && expanded,
          },
          ns.be('node', 'expand-icon'),
        ]"
        @click.stop="handleExpandIconClick"
      >
        <component :is="icon" />
      </el-icon>
      <el-checkbox
        v-if="showCheckbox"
        :model-value="checked"
        :indeterminate="indeterminate"
        :disabled="disabled"
        @change="handleCheckChange"
        @click.stop
      />
      <el-node-content :node="contentNode" />
    </div>
  </div>
</template>

<script lang="ts" setup>
import { computed, inject } from 'vue'
import ElIcon from '@element-plus/components/icon'
import { CaretRight } from '@element-plus/icons-vue'
import ElCheckbox from '@element-plus/components/checkbox'
import { useNamespace } from '@element-plus/hooks'
import { isFunction, isString } from '@element-plus/utils'
import ElNodeContent from './tree-node-content'
import {
  NODE_CONTEXTMENU,
  ROOT_TREE_INJECTION_KEY,
  treeNodeEmits,
  treeNodeProps,
} from './virtual-tree'

import type { CheckboxValueType } from '@element-plus/components/checkbox'
import type { TreeNode } from './types'

defineOptions({
  name: 'ElTreeNode',
})

Review comment (suggestion):
```suggestion
      <el-node-content :node="node" />
```

The `node` already contains the correct `expanded` property.

https://github.com/element-plus/element-plus/blob/a228b3a514106bf6d9025615349fbe0de626cefb/packages/components/tree-v2/src/tree.vue#L21-L22

Code after:
        height: itemSize + 'px',
      }"
    >
      <el-icon
        v-if="icon"
        :class="[
          ns.is('leaf', !!node?.isLeaf),
          ns.is('hidden', hiddenExpandIcon),
          {
            expanded: !node?.isLeaf && expanded,
          },
          ns.be('node', 'expand-icon'),
        ]"
        @click.stop="handleExpandIconClick"
      >
        <component :is="icon" />
      </el-icon>
      <el-checkbox
        v-if="showCheckbox"
        :model-value="checked"
        :indeterminate="indeterminate"
        :disabled="disabled"
        @change="handleCheckChange"
        @click.stop
      />
      <el-node-content :node="{ ...node, expanded }" />
    </div>
  </div>
</template>

<script lang="ts" setup>
import { computed, inject } from 'vue'
import ElIcon from '@element-plus/components/icon'
import { CaretRight } from '@element-plus/icons-vue'
import ElCheckbox from '@element-plus/components/checkbox'
import { useNamespace } from '@element-plus/hooks'
import { isFunction, isString } from '@element-plus/utils'
import ElNodeContent from './tree-node-content'
import {
  NODE_CONTEXTMENU,
  ROOT_TREE_INJECTION_KEY,
  treeNodeEmits,
  treeNodeProps,
} from './virtual-tree'

import type { CheckboxValueType } from '@element-plus/components/checkbox'
import type { TreeNode } from './types'

defineOptions({
  name: 'ElTreeNode',
})
Repository: element-plus/element-plus
File: packages/components/time-select/src/time-select.ts (TypeScript)

Code before:
   * @description same as `name` in native input
   */
  name: String,
  /**
   * @description custom prefix icon component
   */
  prefixIcon: {
    type: definePropType<string | Component>([String, Object]),
    default: () => Clock,
  },
  /**
   * @description custom clear icon component
   */
  clearIcon: {
    type: definePropType<string | Component>([String, Object]),
    default: () => CircleClose,
  },
  ...useEmptyValuesProps,
} as const)

export type TimeSelectProps = ExtractPropTypes<typeof timeSelectProps>
export type TimeSelectPropsPublic = __ExtractPublicPropTypes<
  typeof timeSelectProps
>

export type TimeSelectInstance = InstanceType<typeof TimeSelect> & unknown


Review comment (suggestion):
```suggestion
  popperClass: useTooltipContentProps.popperClass,
```

Code after:
  /**
   * @description whether `end` is included in options
   */
  includeEndTime: Boolean,
  /**
   * @description same as `name` in native input
   */
  name: String,
  /**
   * @description custom prefix icon component
   */
  prefixIcon: {
    type: definePropType<string | Component>([String, Object]),
    default: () => Clock,
  },
  /**
   * @description custom clear icon component
   */
  clearIcon: {
    type: definePropType<string | Component>([String, Object]),
    default: () => CircleClose,
  },
  /**
   * @description custom class name for TimeSelect's dropdown
   */
  popperClass: {
    type: String,
    default: '',
  },
  /**
   * @description custom style for TimeSelect's dropdown
   */
  popperStyle: {
    type: definePropType<string | CSSProperties>([String, Object]),
  },
  ...useEmptyValuesProps,
} as const)

export type TimeSelectProps = ExtractPropTypes<typeof timeSelectProps>
export type TimeSelectPropsPublic = __ExtractPublicPropTypes<
  typeof timeSelectProps
>

export type TimeSelectInstance = InstanceType<typeof TimeSelect> & unknown
Repository: element-plus/element-plus
File: packages/components/autocomplete/src/autocomplete.vue (Vue)

Code before:
    [EVENT_CODE.enter]: () => {
      e.preventDefault()
      handleKeyEnter()
    },
    [EVENT_CODE.tab]: () => {
      close()
    },
    [EVENT_CODE.esc]: () => {
      handleKeyEscape(e)
    },
    [EVENT_CODE.home]: () => {
      highlight(0)
    },
    [EVENT_CODE.end]: () => {
      highlight(suggestions.value.length - 1)
    },
    [EVENT_CODE.pageUp]: () => {
      e.preventDefault()
      highlight(calcPageIndex(-1))
    },
    [EVENT_CODE.pageDown]: () => {
      e.preventDefault()
      highlight(calcPageIndex(1))
    },
  }
  keyHandlers[e.key]?.()
}

onBeforeUnmount(() => {
  stopHandle?.()
})

onMounted(() => {
  const inputElement = inputRef.value?.ref
  if (!inputElement) return
  ;[
    { key: 'role', value: 'textbox' },
    { key: 'aria-autocomplete', value: 'list' },
    { key: 'aria-controls', value: 'id' },
    {
      key: 'aria-activedescendant',
      value: `${listboxId.value}-item-${highlightedIndex.value}`,
    },
  ].forEach(({ key, value }) => inputElement.setAttribute(key, value))
  // get readonly attr
  readonly = inputElement.hasAttribute('readonly')
})

defineExpose({
  /** @description the index of the currently highlighted item */
  highlightedIndex,

Review comment (suggestion):
Perhaps we can refer to #22228, use `getEventCode` to obtain KeyboardEvent.code.

https://github.com/element-plus/element-plus/blob/0e8b01787419c4343c405e673df16c7092849c13/packages/utils/dom/event.ts#L25

Code after:
      highlight(0)
      break
    case EVENT_CODE.end:
      e.preventDefault()
      highlight(suggestions.value.length - 1)
      break
    case EVENT_CODE.pageUp:
      e.preventDefault()
      highlight(Math.max(0, highlightedIndex.value - 10))
      break
    case EVENT_CODE.pageDown:
      e.preventDefault()
      highlight(
        Math.min(suggestions.value.length - 1, highlightedIndex.value + 10)
      )
      break
  }
}

onBeforeUnmount(() => {
  stopHandle?.()
})

onMounted(() => {
  const inputElement = inputRef.value?.ref
  if (!inputElement) return
  ;[
    { key: 'role', value: 'textbox' },
    { key: 'aria-autocomplete', value: 'list' },
    { key: 'aria-controls', value: 'id' },
    {
      key: 'aria-activedescendant',
      value: `${listboxId.value}-item-${highlightedIndex.value}`,
    },
  ].forEach(({ key, value }) => inputElement.setAttribute(key, value))
  // get readonly attr
  readonly = inputElement.hasAttribute('readonly')
})

defineExpose({
  /** @description the index of the currently highlighted item */
  highlightedIndex,
  /** @description autocomplete whether activated */
  activated,
  /** @description remote search loading status */
  loading,
  /** @description el-input component instance */
  inputRef,
  /** @description el-tooltip component instance */
  popperRef,
  /** @description fetch suggestions result */
Repository: element-plus/element-plus
File: packages/directives/repeat-click/index.ts (TypeScript)

Code before:
      handler()

      document.addEventListener('mouseup', clear, { once: true })

      delayId = setTimeout(() => {
        intervalId = setInterval(() => {
          handler()
        }, interval)
      }, delay)
    }

    el[SCOPE] = { start, clear }
    el.addEventListener('mousedown', start)
  },
  unmounted(el) {
    if (!el[SCOPE]) return
    const { start, clear } = el[SCOPE]

    if (start) {
      el.removeEventListener('mousedown', start)
    }
    if (clear) {
      clear()
      document.removeEventListener('mouseup', clear)
    }
    el[SCOPE] = {}
  },
}


Review comment (suggestion):
```suggestion
    el[SCOPE] = null
```
It seems unnecessary to assign an empty object.

Code after:
      handler()

      document.addEventListener('mouseup', clear, { once: true })

      delayId = setTimeout(() => {
        intervalId = setInterval(() => {
          handler()
        }, interval)
      }, delay)
    }

    el[SCOPE] = { start, clear }
    el.addEventListener('mousedown', start)
  },
  unmounted(el) {
    if (!el[SCOPE]) return
    const { start, clear } = el[SCOPE]

    if (start) {
      el.removeEventListener('mousedown', start)
    }
    if (clear) {
      clear()
      document.removeEventListener('mouseup', clear)
    }
    el[SCOPE] = null
  },
}
Repository: element-plus/element-plus
File: packages/components/select/src/useSelect.ts (TypeScript)

Code before:
        navigateOptions('prev')
        break
      case EVENT_CODE.down:
        navigateOptions('next')
        break
      case EVENT_CODE.enter:
        selectOption()
        break
      case EVENT_CODE.esc:
        handleEsc()
        break
      case EVENT_CODE.backspace:
        isPreventDefault = false
        deletePrevTag(e)
        return
      case EVENT_CODE.home:
        if (!expanded.value) return
        focusOption(0, 'down')
        break
      case EVENT_CODE.end:
        if (!expanded.value) return
        focusOption(states.options.size - 1, 'up')
        break
      case EVENT_CODE.pageUp:
        if (!expanded.value) return
        focusOption(Math.max(0, states.hoveringIndex - 10), 'up')
        break
      case EVENT_CODE.pageDown:
        if (!expanded.value) return
        focusOption(
          Math.min(states.options.size - 1, states.hoveringIndex + 10),
          'down'
        )
        break
      default:
        isPreventDefault = false
        break
    }
    if (isPreventDefault) {
      e.preventDefault()
      e.stopPropagation()
    }
  }

  const getGapWidth = () => {
    if (!selectionRef.value) return 0
    const style = window.getComputedStyle(selectionRef.value)
    return Number.parseFloat(style.gap || '6px')
  }

  // computed style

Review comment (suggestion):
Since the clamp( `Math.min(Math.max(Math.floor(targetIndex), 0), len - 1)`) function is already used in the `focusOption` method, it doesn’t seem necessary to add it here.

```suggestion
        focusOption(states.hoveringIndex - 10, 'up')
```

Code after:
      case EVENT_CODE.backspace:
        isPreventDefault = false
        deletePrevTag(e)
        return
      case EVENT_CODE.home:
        if (!expanded.value) return
        focusOption(0, 'down')
        break
      case EVENT_CODE.end:
        if (!expanded.value) return
        focusOption(states.options.size - 1, 'up')
        break
      case EVENT_CODE.pageUp:
        if (!expanded.value) return
        focusOption(states.hoveringIndex - 10, 'up')
        break
      case EVENT_CODE.pageDown:
        if (!expanded.value) return
        focusOption(states.hoveringIndex + 10, 'down')
        break
      default:
        isPreventDefault = false
        break
    }
    if (isPreventDefault) {
      e.preventDefault()
      e.stopPropagation()
    }
  }

  const getGapWidth = () => {
    if (!selectionRef.value) return 0
    const style = window.getComputedStyle(selectionRef.value)
    return Number.parseFloat(style.gap || '6px')
  }

  // computed style
  const tagStyle = computed(() => {
    const gapWidth = getGapWidth()
    const inputSlotWidth = props.filterable ? gapWidth + MINIMUM_INPUT_WIDTH : 0
    const maxWidth =
      collapseItemRef.value && props.maxCollapseTags === 1
        ? states.selectionWidth -
          states.collapseItemWidth -
          gapWidth -
          inputSlotWidth
        : states.selectionWidth - inputSlotWidth
    return { maxWidth: `${maxWidth}px` }
  })

  const collapseTagStyle = computed(() => {