Enhance focus on close buttons (fixes #1079)#1147
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
demiankatz
left a comment
There was a problem hiding this comment.
@Geoffsc, this looks good, but it appears that your solution relies on Tailwind classes. While Tailwind is loaded as part of the UV examples, it is not actually part of the UV itself, so I don't think this will work when the UV is used in other contexts.
I'm not totally sure what a more robust solution would be, but I wonder if it would help to define .btn.close styles in the dialogues module's LESS file.
demiankatz
left a comment
There was a problem hiding this comment.
Thanks for the refactor, @Geoffsc, I think this should work more consistently across the board. Just two remaining questions:
1.) Is it just me, or does the button focus indicator seem a lighter color than everything else? I'd describe it as more of a periwinkle than a blue...
2.) Did you try using a .btn.close selector instead of .button-close so that you don't have to change the classes in the Typescript files at all? I realize that maybe you intentionally got rid of the original btn btn-default close classes to avoid inheriting unwanted styles... but I thought I'd ask just in case, because if we could do it that way, then we wouldn't have to change as many files, and there'd be less risk of breaking other things tied to the pre-existing classes (if any such things exist that we don't already know about).
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @Geoffsc, this looks good to me!
Not sure if @crhallberg or others might have suggestions for further optimizing/simplifying the LESS, but it clearly works as intended as-is.
Maybe @LanieOkorodudu or others can double-check my testing next week before we merge this!
|
@demiankatz Sounds good! |
|
@demiankatz, I have tested the enhancement, and it works as expected. The close button now correctly receives focus when clicked, ensuring that users are visually aware of the active focus state on the button. Thanks @Geoffsc |
Fix for #1079