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

[BUG] Accesibility Issue with Icon Dropdown #9707

Open
1 task done
at-the-vr opened this issue Oct 31, 2023 · 10 comments · May be fixed by #9913
Open
1 task done

[BUG] Accesibility Issue with Icon Dropdown #9707

at-the-vr opened this issue Oct 31, 2023 · 10 comments · May be fixed by #9913
Assignees
Labels
🛠 goal: fix undefined 🔢 points: 2 undefined 🏁 status: ready for dev You can asked for this issue to be assigned (if not already assigned)

Comments

@at-the-vr
Copy link

Has this bug been raised before?

  • I have checked "open" AND "closed" issues and this is not a duplicate

Where did you find this bug?

production

Version of BioDrop (for example "v1.2.3")

v2.83.18

Description

The Icon Dropdown under AddLink & Add Milestone do not respond to Arrow Key Presses.

Screenshots

No response

Do you want to work on this issue?

No

If you want to work on this issue...

No response

@at-the-vr at-the-vr added 🚦 status: awaiting triage Waiting for maintainers to verify (please do not start work on this yet) 🛠 goal: fix undefined labels Oct 31, 2023
Copy link
Contributor

To reduce notifications, issues are locked until they are 🏁 status: ready for dev You can asked for this issue to be assigned (if not already assigned) and to be assigned. You can learn more in our contributing guide https://github.com/EddieHubCommunity/BioDrop/blob/main/CONTRIBUTING.md

@github-actions github-actions bot locked and limited conversation to collaborators Oct 31, 2023
@EddieHubCommunity EddieHubCommunity unlocked this conversation Nov 19, 2023
@SaraJaoude
Copy link
Member

Thank you for raising. I have added the label talk: discussion so that the community can add their comments on how this can be resolved.

@aryanraj60
Copy link

aryanraj60 commented Dec 9, 2023

I have fixed this issue, can you please assign this issue to me so that I can make a pull request to close this.. @SaraJaoude

This issue can we solved just by 3-4 lines of code

const [highlightedIndex, setHighlightedIndex] = useState(0);

First, I added a highlightedIndex state to keep track of the currently highlighted option.

 onKeyDown={(event) => {
          if (event.key === "ArrowDown" && highlightedIndex < filteredIcon.length - 1) {
            setHighlightedIndex((prevIndex) => prevIndex + 1);
          } else if (event.key === "ArrowUp" && highlightedIndex > 0) {
            setHighlightedIndex((prevIndex) => prevIndex - 1);
          }
 }}

The onKeyDown event handler is added to the Combobox.Input to handle arrow key presses. It updates the highlightedIndex accordingly.

className={`px-3 py-2 flex items-center ${
                highlightedIndex === index
                  ? "bg-red-400"
                  : "dark:hover:bg-tertiary-medium/60 hover:bg-secondary-low/40"
              }`}

The Combobox.Option component is modified to apply a different style to the highlighted option.

biodrop-keypress-feature.mov

@SaraJaoude
Copy link
Member

Thank you @aryanraj60 - we are going to review this. In future, please can you avoid tagging a specific maintainer as this creates extra notifications.

@EmmaDawsonDev and @YuriDevAT if either of you have a moment, we would love to get your thoughts on this.

@YuriDevAT
Copy link
Member

YuriDevAT commented Dec 11, 2023

Thanks for asking! As far as I can see, all it needs is to add background-color to the active item.

When you look at the DOM, you see that you CAN select the items by using the arrow keys up/down (data-headlessui-state="" changes its value from "" to active), but it is not visible to the user.

As I see from the DOM, this combobox is a headless UI combobox - https://headlessui.com/react/combobox and there, it works perfectly fine. So my guess is that something went wrong by the implementation, and it was forgotten to set a background color for the active state, like they do here at headlessui.

className={({ active }) =>
  `relative cursor-default select-none py-2 pl-10 pr-4 ${
    active ? 'bg-teal-600 text-white' : 'text-gray-900'
  }`
}

Unfortunately they do not have line numbers in the very first example, but this code snippet can be found in the middle of the return statement.

I hope this helps.

Note: My insights are based solely on what I identified in the DOM and headlessui combobox example.

@eddiejaoude
Copy link
Member

eddiejaoude commented Dec 11, 2023

Amazing explanation @YuriDevAT ! Thank you for spending time and looking into it

ℹ️ eddiejaoude has some opened assigned issues: 🔧View assigned issues

@eddiejaoude
Copy link
Member

eddiejaoude commented Dec 12, 2023

@aryanraj60 please see response above, I think your solution can be simplified #9707 (comment)

ℹ️ eddiejaoude has some opened assigned issues: 🔧View assigned issues

@aryanraj60
Copy link

aryanraj60 commented Dec 13, 2023

Thanks for your suggestion @YuriDevAT, Yes eddiejaoude, I checked and tested her suggestion and it's works fine just by adding this code to Combobox.Option -

className={({ active }) =>
                `px-3 py-2 flex items-center ${
                  active
                    ? "bg-red-400"
                    : "dark:hover:bg-tertiary-medium/60 hover:bg-secondary-low/40"
                }`
              }

@SaraJaoude
Copy link
Member

@aryanraj60 please see my comments here.

On this occasion we will assign this issue to you and whilst we appreciate your enthusiasm, in future please follow this project's requirements and guidelines.

@SaraJaoude SaraJaoude added 🏁 status: ready for dev You can asked for this issue to be assigned (if not already assigned) 🔢 points: 2 undefined and removed 🚦 status: awaiting triage Waiting for maintainers to verify (please do not start work on this yet) 💬 talk: discussion undefined labels Dec 18, 2023
Copy link
Contributor

The issue has been unlocked and is now ready for dev. If you would like to work on this issue, you can comment to have it assigned to you. You can learn more in our contributing guide https://github.com/EddieHubCommunity/BioDrop/blob/main/CONTRIBUTING.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠 goal: fix undefined 🔢 points: 2 undefined 🏁 status: ready for dev You can asked for this issue to be assigned (if not already assigned)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants