Skip to content

show focus outlines #490

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

Merged
merged 3 commits into from
Feb 5, 2025
Merged

show focus outlines #490

merged 3 commits into from
Feb 5, 2025

Conversation

rubys
Copy link
Contributor

@rubys rubys commented Feb 4, 2025

Fixes #489

@patriciomacadden
Copy link
Contributor

Maybe worth doing the same to the new session view?

otherwise they look different:

passwords:
image

new session:
image

the new session looks brighter because it doesn't have the focus:outline-solid class.

What I'm not sure is what looks better, if no focus:outline-solid and no outline-hidden at all or just focus:outline-solid.

what do you think?

@rubys
Copy link
Contributor Author

rubys commented Feb 4, 2025

Other than "they probably should be consistent", I have no opinion :-)

I'm just trying to update Agile Web Development with Rails 8, and would like to get an idea of what this will ultimately look like.

@patriciomacadden
Copy link
Contributor

yep, I agree they should be consistent.

Perhaps just removing outline-hidden is one less class and makes the outline brighter, but I'm fine with either option. I'll leave this decision to you and @flavorjones .

PS: I love Agile web development with Rails! Thanks!

@flavorjones
Copy link
Member

Thanks to you both for chatting about this! I personally like the "brighter" style of the sessions form which omits focus:outline-solid (but that may be because I'm color-blind and need to be hit over the head with affordances like this).

I'll push an update to this PR to make that change and then merge it.

because I like the brighter focus highlight, especially if it's used
to indicate fields with validation errors.
@flavorjones flavorjones merged commit c9f3ecb into rails:main Feb 5, 2025
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

Successfully merging this pull request may close these issues.

field outlines are hidden - even when focus is applied.
3 participants