Add radio role and aria checked state to color checkboxes

[RoyEJohnson]

DISCO-323
Leaving the type as checkbox retained the functionality. Adding the role takes care of the accessibility.

[jivey]

Wouldn't a radio type work? From what I can tell, once one is checked, there is no unchecked state, so this seems like a clear case to me that these should be radios...

[RoyEJohnson]

That's exactly why a radio doesn't work: we need to be able to deselect the selected color to remove the highlight.

[jivey]

Sorry I missed your follow up message last week. Doesn't role="radio" mean the screenreader won't be informed there is a possibility to uncheck it? Usually re-checking a checked radio does nothing, so this seems like a hidden, surprise interaction. With the uncheck-deletion only occurring on unsaved highlights, it feels like we should move that interaction to a Cancel button instead. Maybe we can check with LA about this?

[RoyEJohnson]

It's definitely worth asking LA about.
Another approach that worked (I noted on the issue card) was to change the type of the selected radio item to checkbox. But that also seemed weird.
Probably the right answer is a remove-highlight button.

[jivey]

(Just summarizing our discussion in a review comment.) Based on the card comments, it looks like VC thinks a remove highlight button it could work. Like you mentioned, we can also check with LA about some other solution that keeps the existing design.

[RoyEJohnson]

I have added a trash icon to the color picker.
I ran into CI issue with our use of actions/upload-artifact v2. Tom suggested I simply remove those bits, so I did.

[jivey]

The trash can seems great, just wondering about it not doing anything in some states.

[jivey]

This is working great in the scenario we discussed. But should it also work before color selection, and after note creation? When I was testing those, nothing happened when I clicked the button. Maybe we just hide it if onRemove isn't set?

[RoyEJohnson]

I had to fiddle with useOnRemove so it would return null instead of a function that did nothing, but that makes it much less confusing to only have the trash can show up when it's functional.