-
Notifications
You must be signed in to change notification settings - Fork 94
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
[BITV] 9.1.4.1/3.1 - Styling menubar - active contrast #3860
[BITV] 9.1.4.1/3.1 - Styling menubar - active contrast #3860
Comments
@jancborchardt I remember we had discussed the current style, but to match the constrast we'd need to use rather black as the color for the active indicator. Shall we just switch to the secondary style as the active indicator or would you have any other suggestion? For comparison the NcButton component, but they don't have dedicated active states |
@jancborchardt what do you think about #3860 (comment) - pending decision is a blocker for resolving the issue by the devs. However contrast is too low. So everything works here except secondary style. |
Yes, that sounds the most reasonable and consistent. |
@jancborchardt So everything works here except secondary style. So that can't be used... |
@AndyScherzinger Why does it not work? The contrast as used for the secondary button is even triple-A compliant: |
@jancborchardt And which possibility do we have for the dark mode? |
Then we either need to use the primary style for highlighting (which might be a bit much), or we do it again like we had some time before with the small dot below the active elements. @juliushaertl do we still have CSS for that by any chance? And if you know what I mean, do you have a preference? I think the version with the dot below the active icon probably looks a bit cleaner. |
I would prefer the primary then, as the dot is not very common among other editing tools and I think we removed it in the past because it was not easy to discover and looked more like a notification. I've also looked at other tools and noticed that except apple, all seem to stick to a light background with doesn't match contrast requirements. Could we come up with a global definition of how active style of a button or element is indicated? The current style is also used in other places or there are other parts where the contrast doesn't match in a similar way:
|
@juliushaertl you're right, for active app nav and app content list we already are switching to primary style as per @JuliaKirschenheuter, then we should indeed do it here too, and for any other active style. |
@jancborchardt ok, does it means that all active elements like would have styles from primary (button), like right? Another alternative would be to use high contrast border. What would you prefer? |
@JuliaKirschenheuter yes, we should use primary style as indicator for active items. This is what e.g. iOS Notes does as well, as @juliushaertl mentioned. Additionally but separately, we need to fix our secondary button style as it's not accessible. If we look at Material Design as reference, they have:
@nextcloud/designers what do you think? I'd say just moving to outline style for secondary button is simplest and safest. |
If i understood @michaelnissenbaum correct: it depends on a context. An active button have to have min. 3:1 contrast. In which cases are we using our secondary buttons? If we would use "4 Outlined button" as secondary button - that would be accessible, yes. |
by the way, Material Design |
Yep exactly. So we should move our secondary button from tonal to outlined, so that when we use it, it is accessible. :) |
As mentioned in chat outline is not very clear to differentiate then to the focus state. @jancborchardt I think we need the design team here to jump in and bring up mockups as clear definition for the following buttons adding additional states. Might be enough to define those for tertiary buttons for the use cases I know of. States we haveNormalDisabledFocusStates we'd need definition
References of existing usages of this caseTextCollaboraAppNavigationItem (not a button but should probably use a similar style) |
Do we have final decision and a summary of it here? |
Still up for discussion, let's wait for feedback from designers on this one. |
this is done |
@JuliaKirschenheuter We would still need to switch to a different active style, right? We can take care of that using nextcloud-libraries/nextcloud-vue#4344, but maybe you can quickly confirm that this is required. |
@JuliaKirschenheuter is this fixed? I thought only the focus issues was solved, what about the active state? |
For many elements like "B" and "I", selected styles are visually marked by a pale blue background (1.5:1 to white) and a change of text color (black to blue, 2.2:1). Both remain below the contrast difference of 3:1 that is needed to indicate the state visually. (2)
Details
https://report.bitvtest.de/default-en/d63601ac-cb34-4645-8256-66bec78964a0.html#checkpoint-45ae33ca9f-v3-n1
The text was updated successfully, but these errors were encountered: