Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🎸 feat: a11y に対応した combobox の作成 #362

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

anko9801
Copy link
Contributor

@anko9801 anko9801 commented Dec 29, 2024

PR Type

Enhancement


Description

  • InputSelectMultiple.vueInputSelectSingle.vueのリファクタリングを実施し、コードの可読性とメンテナンス性を向上。
  • キーボード操作の処理を改善し、switch文を導入。
  • 新しいnavigateList関数を追加してリストナビゲーションのロジックを分離。
  • ドロップダウンの開閉処理をopenDropdown関数に統一。
  • アクセシビリティ向上のため、role="button"tabindex="0"を追加。

Changes walkthrough 📝

Relevant files
Enhancement
InputSelectMultiple.vue
入力選択コンポーネントのリファクタリングとキーボード操作の改善                                                   

src/components/shared/InputSelectMultiple.vue

  • リファクタリング: 関数を分割し、コードの可読性を向上。
  • キーボード操作の処理をswitch文に変更し、コードの簡潔性を向上。
  • 新しいnavigateList関数を追加し、リストナビゲーションのロジックを分離。
  • ドロップダウンの開閉処理をopenDropdown関数に統一。
  • +62/-68 
    InputSelectSingle.vue
    シングル選択コンポーネントのリファクタリングとアクセシビリティ改善                                               

    src/components/shared/InputSelectSingle.vue

  • リファクタリング: ドロップダウンの開閉処理をopenDropdown関数に統一。
  • 不要なcalcWidth計算を削除。
  • アクセシビリティ向上のため、role="button"tabindex="0"を追加。
  • +8/-10   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Possible Issue

    The navigateList function assumes that listItemRefs.value is not null, but there is no null check before accessing listItemRefs.value[focusingListItemIndex.value]. This could lead to runtime errors if listItemRefs.value is null.

    const navigateList = (direction: 'up' | 'down') => {
      if (listItemRefs.value === null) return
    
      const length = searchedOptions.value.length
      const increment = direction === 'down' ? 1 : -1
      focusingListItemIndex.value =
        (focusingListItemIndex.value + increment + length) % length
    
      const buttonEl = listItemRefs.value[focusingListItemIndex.value]
        .firstChild as HTMLButtonElement
      buttonEl?.focus({ preventScroll: false })
    Accessibility Issue

    The role="button" and tabindex="0" attributes are added, but there is no explicit aria-label or aria-labelledby to describe the button's purpose for screen readers.

    :class="`relative ${disabled && 'cursor-not-allowed'}`">
    <div
      role="button"
      tabindex="0"
      class="flex cursor-text items-center w-full py-1 pl-1 rounded border border-surface-secondary"
      :class="`${disabled && 'pointer-events-none'}`"
      @click.prevent="openDropdown"
      @keydown.enter.prevent="openDropdown"
      @keydown.space.prevent="openDropdown">

    @anko9801 anko9801 self-assigned this Dec 29, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant