React95 icon indicating copy to clipboard operation
React95 copied to clipboard

[WIP] SelectBox

Open pxd3v opened this issue 3 years ago β€’ 10 comments

  • Creates select box component https://github.com/arturbien/React95/issues/123 image

pxd3v avatar Oct 16 '20 01:10 pxd3v

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

πŸ” Inspect: https://vercel.com/arturbien/react95/m09w3ieh0
βœ… Preview: https://react95-git-master.arturbien.vercel.app

vercel[bot] avatar Oct 16 '20 01:10 vercel[bot]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f8f279371431d6971ac9694d0b20f6ff77caa10e:

Sandbox Source
React95 template Configuration

codesandbox-ci[bot] avatar Oct 16 '20 01:10 codesandbox-ci[bot]

@pxd3v hey! first of all thanks for your work β™₯️ there are couple of issues I've noticed in the component tho:

  1. There's some excess border inside the Cutout Screenshot 2020-10-18 at 13 25 25

Here's the Cutout for comparison: Screenshot 2020-10-18 at 13 28 00

  1. When controlling the component with a keyboard, scrolling should behave exactly the same as in Select component. Currently the behaviour is different. When you select second option, it's scrolled to the top of the component so the first option is no longer visible. Also we don't want the 'smooth' scrolling

  2. Component should use the same font other components use

  3. We shouldn't use button elements in the SelectBox. You can use this guide for reference: https://www.w3.org/TR/2017/WD-wai-aria-practices-1.1-20170628/examples/listbox/listbox.html . They're simply using ul and li elements with proper aria-labels for accessibility. That's how we also did it in the Select component

arturbien avatar Oct 18 '20 11:10 arturbien

Hi @arturbien! Thank you again for the guidance. I just fixed some issues that you've pointed out, in this commit: https://github.com/arturbien/React95/pull/197/commits/89472e29aad6d7a60f6cadfdad2bcca435b86734. About the aria-labels, i will study it in order to apply it so we can improve acessibility! Have a nice sunday.

pxd3v avatar Oct 18 '20 13:10 pxd3v

@pxd3v thanks! borders and fonts look good now :)

One more style change that comes to my mind- can you change option height to 31px ? Options in Select dropdown have that height and I think it's the optimal size.

Scrolling now works as expected, but I've noticed that it's not possible to set focus to the SelectBox before clicking any of the options. I think it's some kind of regression because previously when I tested it it worked. Can you check what happens there?

arturbien avatar Oct 18 '20 19:10 arturbien

@arturbien I'm glad you like it!

So, I don't know if i understood the SelectBox focus thing, you want the user to have to double click the SelectBox at first? That way he can firstly focus on the selectBox and then focus an option?

Ah, i just commited a new test and the option size fix :)

pxd3v avatar Oct 19 '20 23:10 pxd3v

@pxd3v now it looks waaay better! ☺️ What I mean is right now it's NOT possible to focus SelectBox using TAB. User should be able to control the component using only keyboard. Right now user has to click the SelectBox first to be able to control it via keyboard

arturbien avatar Oct 20 '20 10:10 arturbien

@arturbien got it! Just fixed this issue.

This week i will try to work on some tests, aria-labels and i will also try to study the "flat" display thing!

pxd3v avatar Oct 20 '20 16:10 pxd3v

Hi Guys! This week I've started working in a new company and because of that I'm not having much free time to work on other projects.. I would like to say that I really enjoyed working on this issue but, for now, I will need to pass it to another developer.

Thank you all so much for the attention this whole time and keep going with this amazing project!

pxd3v avatar Nov 04 '20 20:11 pxd3v

@pxd3v no worries. thank you for your work, we will take on from here and finish it.

Also congratulations and good luck with your new job! <3

arturbien avatar Nov 04 '20 21:11 arturbien