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

Focus styles #1

Closed
devongovett opened this issue Jul 19, 2023 · 6 comments
Closed

Focus styles #1

devongovett opened this issue Jul 19, 2023 · 6 comments

Comments

@devongovett
Copy link

First off great job with this library, it looks super awesome. Love the idea of using a VSCode extension to bootstrap components! 🤩

I noticed a few of the components are missing keyboard focus styles. That's an important accessibility feature for folks navigating with the keyboard to see where they are. Most of the React Aria Components include a data-focus-visible attribute that you could use with the tailwind ring classes to implement a focus ring.

Another related case is with the menu/dropdown/combobox/etc. components I saw you're currently using tailwind's hover: modifier. You might want to consider using the data-focused attribute for that instead, which gets set for both hover and focus. And for combobox it also works even though actual DOM focus stays in the input field. Right now when you navigate via the arrow keys you can't see which item is focused.

In general our data attributes are designed to be more consistent cross browser and device than the css counterparts like hover: and active:, so you might want to consider using them everywhere as well. You can read our blog post series on this if you wanna learn more about why, and find all the available states in the docs.

Again, awesome work on this! Really excited to see where it goes.

@zwgnr
Copy link
Owner

zwgnr commented Jul 19, 2023

Thanks for the kind words and all the thoughtful feedback!

The focus/hover styling is something that has been on my to do list to improve. While building I was running into some issues with the data-focus-visible/data-hover and Tailwind.

Take the Button component for example. I could not get [&[data-focus-visible]]:ring-2 to work. It’s odd because other data attributes work fine this way such as things like [&[data-outside-month]]:hidden in the Calendar.

I suppose I could pass isFocusVisible as a variant prop, then make a “focused” variant. However I was trying to avoid that for brevity, if possible.

Any reason why the css selector version is not working for things like data-focus-visible or data-hovered, but is working for things like data-outside-month?

@devongovett
Copy link
Author

hmm not sure. I usually use the data-[focus-visible]: syntax, but I would have thought the one you showed here would work too? I do see the data-focus-visible attribute getting applied in the DOM so not sure why tailwind wouldn't pick it up. I definitely have a bunch of examples where it works. Maybe try the data attribute syntax instead of fully custom arbitrary selectors and see if that helps?

@zwgnr
Copy link
Owner

zwgnr commented Jul 20, 2023

okay I got it sorted. The issue was an over site on my part as a result of missing an Astro client directive . I was doing the initial data-focus tests with a button component which as it turns out, in the location I was testing, was missing the proper client directive. 😅 This resulted in the data-focus-visible attributes actually not being set in the DOM.

Everything is working smoothly now. I am in the process of refactoring and migrating from the tailwind modifiers to the provided Aria data states!

@roguesherlock
Copy link

roguesherlock commented Jul 20, 2023

btw if you want to apply styles to the children based on the data attributes, you can do it with group class from tailwind.
The way I currently do in my components is that I add a group class to the root of the component where the data-attributes are going to be set, and then on the children elements I use group-data-[state=open]:<your-class> to apply styles based on data attribute.

P.S. btw I love your vscode extension approach. It's pretty neat.

@zwgnr
Copy link
Owner

zwgnr commented Jul 20, 2023

going to close this. Just merged a ton of upgrades. I changed all the focus states to data-focus-visible or data-focused, as well as upgraded all the tailwind modifiers (hover, active) to the relative Aria data-states.

@devongovett Thanks again for your attention on this, everything should be much improved now!

@zwgnr zwgnr closed this as completed Jul 20, 2023
@devongovett
Copy link
Author

Nice looks like it is working well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants