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

Fix predefined status buttons #39987

Merged
merged 1 commit into from Aug 22, 2023

Conversation

JuliaKirschenheuter
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter commented Aug 21, 2023

Summary

Before After
Peek 2023-08-21 14-24 Peek 2023-08-22 13-16

Checklist

@jancborchardt
Copy link
Member

@marcoambrosini what do you think of the style?

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the border is fine, but I think we should put the radio button symbol too @JuliaKirschenheuter
I'm afraid that we're confusing people a lot with these radio buttons that don't look like radios.

@JuliaKirschenheuter
Copy link
Contributor Author

JuliaKirschenheuter commented Aug 22, 2023

I'm afraid that we're confusing people a lot with these radio buttons that don't look like radios.

yes, i'm agree that this buttons are not really radio buttons. But let them be radio buttons for now. We had only an issue that they are not really recognizable.

I think that the border is fine, but I think we should put the radio button symbol too

I think then it will be lots of "flashing elements". A11y-wise it is not required.

If border is ok - could we leave it like this for now? All improvements we can do afterwards

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JuliaKirschenheuter let's add a normal border radius and get this in, going forward I think we should add the radio button (which is now .hidden-visually) to all radio buttons

Screenshot 2023-08-22 at 19 47 52

@JuliaKirschenheuter
Copy link
Contributor Author

@JuliaKirschenheuter let's add a normal border radius and get this in

do you mean the same border radius like in "online status"?

@marcoambrosini
Copy link
Member

do you mean the same border radius like in "online status"?

Yes @JuliaKirschenheuter var(--border-radius-large)

Add styles for radio buttons to be visible in checked, active and focus-visible state.

Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/39926-fix_predefined_status-buttons branch from ae7e7f1 to 558f5f9 Compare August 22, 2023 11:23
@JuliaKirschenheuter JuliaKirschenheuter merged commit e42d82f into master Aug 22, 2023
41 checks passed
@JuliaKirschenheuter JuliaKirschenheuter deleted the fix/39926-fix_predefined_status-buttons branch August 22, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BITV] Increase contrast on focus state of "Status Message"
4 participants