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 look of inactive settings tab icons #457

Merged
merged 4 commits into from
Oct 24, 2022
Merged

Fix look of inactive settings tab icons #457

merged 4 commits into from
Oct 24, 2022

Conversation

chrisgrieser
Copy link
Contributor

@chrisgrieser chrisgrieser commented Oct 23, 2022

the name of the inactive tab is not properly hidden, causing issues with some themes like mine. This PR adds a rule which should properly fix this issue for any theme.

without the PR:
Pasted image 2022-10-23 19 46 03

with the PR:
Pasted image 2022-10-23 19 47 43

@pjkaufman
Copy link
Collaborator

@chrisgrieser, thanks for pointing this out! Is there a reason to not use the hidden class? Display none works, but is not accessibility friendly.

@chrisgrieser
Copy link
Contributor Author

chrisgrieser commented Oct 24, 2022

What do you mean by "hidden class"? If you mean visibility: hidden;, yeah that does work, too, I didn't know that this makes a difference accessibility wise. Have changed the PR accordingly.

@pjkaufman
Copy link
Collaborator

What do you mean by "hidden class"? If you mean visibility: hidden;, yeah that does work, too, I didn't know that this makes a difference accessibility wise. Have changed the PR accordingly.

There is a style called hidden which is not perfect, but it should be more accessibility friendly.

@pjkaufman
Copy link
Collaborator

I had the name wrong. Here is the style:

.linter-visually-hidden {

@chrisgrieser
Copy link
Contributor Author

well, since the inactive tab names do not get that hidden class, I assume that would have to be done via js, and not via css, and therefore outside my area of expertise 😝

although if I understand the article correctly, opacity: 0 would also work to hide it while keeping it there fore screenreaders etc.

@pjkaufman
Copy link
Collaborator

I could be wrong, but why would it not be feasible to extend visually hidden with the css selector you had? It should be able to apply to both based on what you are saying.

@chrisgrieser
Copy link
Contributor Author

to my knowledge, there is no way in (pure) css to extend the styling of one class to another?

@chrisgrieser
Copy link
Contributor Author

ah, now I understand what you mean. Yeah, adding the selector to that block also works, sure. done so in the latest commit

@pjkaufman
Copy link
Collaborator

That's what I was going for. Thanks!

@pjkaufman pjkaufman merged commit 2173461 into platers:master Oct 24, 2022
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.

None yet

2 participants