Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Hiding colorpicker input from tab navigation #1791

Merged
merged 12 commits into from
Jul 30, 2018

Conversation

blackbaud-conorwright
Copy link
Contributor

Hid colorpicker input from tab navigation so that it can't be directly modified

Resolves: #1384

@codecov-io
Copy link

codecov-io commented Jul 2, 2018

Codecov Report

Merging #1791 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1791      +/-   ##
=========================================
+ Coverage   99.98%    100%   +0.01%     
=========================================
  Files         412     412              
  Lines        8610    8610              
  Branches     1271    1271              
=========================================
+ Hits         8609    8610       +1     
+ Misses          1       0       -1
Impacted Files Coverage Δ
...modules/colorpicker/colorpicker-input.directive.ts 100% <100%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10c7905...afcc43d. Read the comment docs.

@Blackbaud-SteveBrush Blackbaud-SteveBrush changed the title hiding colorpicker input from tab navigation [HOLD] hiding colorpicker input from tab navigation Jul 3, 2018
@Blackbaud-SteveBrush
Copy link
Member

@Blackbaud-MattGregg This pull request, and #1788 could be in conflict with one another. If we cannot reach the native input element with a keyboard, will a screen reader be able to read out the input's aria-label? See: https://github.com/blackbaud/skyux2/pull/1788/files#diff-bd971b56c22172aefe158a92492c557cR137

@Blackbaud-MattGregg
Copy link
Contributor

@Blackbaud-SteveBrush yes, that is an issue plus someone using a screenreader being able to understand this is an input field asking for color value input. They could also just type it in. It's pretty similar to our data/time pickers.
I'm not sure where the issue of someone being able to modify the input came from as it's not something called out in #1384. Is this problematic for some other reason?
It looks like the button to open the color picker now has visual focus indicated and the input field takes keyboard input but it's wonky or only takes 3 characters?
One suggestion for a different fix for this would be to use a readonly and aria-readonly attributes on the input field instead (both b/c I don't know browser/assistive tech support). This should restrict input and still allow it to be read out. It will still be in tab order and receive focus, but I think that would be fine. We'd just be restricting keyboard users to input using the picker modal and not being able to use the keyboard for direct input.

@blackbaud-conorwright
Copy link
Contributor Author

@Blackbaud-MattGregg I believe the issue of someone not being allowed to modify the input field directly could have been a misunderstanding on my part. in #1384 the expected section says:

No visual indication occurs when on input element which can't take any direct input.`

Which I interpreted to mean that this field was expected to not be able to take any direct input.

@blackbaud-conorwright
Copy link
Contributor Author

Instead of being hidden the input field is going to now be readonly

@blackbaud-conorwright blackbaud-conorwright changed the title [HOLD] hiding colorpicker input from tab navigation Hiding colorpicker input from tab navigation Jul 16, 2018
@Blackbaud-SteveBrush Blackbaud-SteveBrush merged commit 01e1b69 into master Jul 30, 2018
@Blackbaud-SteveBrush Blackbaud-SteveBrush deleted the fix-colorpicker-input-access branch July 30, 2018 15:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants