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

Reduce selector specificity in _header.scss #9789

Open
mateusbandeiraa opened this issue Oct 1, 2021 · 2 comments
Open

Reduce selector specificity in _header.scss #9789

mateusbandeiraa opened this issue Oct 1, 2021 · 2 comments
Labels
component: ui-shell needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. proposal: accepted This request has gone through triaging and we are accepting PR's against it. role: dev 🤖 type: a11y ♿ type: enhancement 💡

Comments

@mateusbandeiraa
Copy link

Is it really necessary to select only icon buttons with class .#{$prefix}--tooltip__trigger?

.#{$prefix}--header__action.#{$prefix}--btn--icon-only.#{$prefix}--tooltip__trigger {
justify-content: center;
}

As the selector is right now, it won't select icon-only buttons that don't have a tooltips. E.g:
image
Left button doesn't have a tooltip, right button has a tooltip.

Can we change the selector to .#{$prefix}--header__action.#{$prefix}--btn--icon-only?

@tay1orjones
Copy link
Member

icon-only buttons that don't have a tooltips

To my knowledge, icon only buttons are intended to always have tooltips as an accessibility affordance. I'm not opposed to decreasing the selector specificity, but for context this was added as a visual fix as part of #7736 that we'll need to ensure still works as intended.

@mateusbandeiraa
Copy link
Author

I agree that in most cases icon-only buttons should have tooltips. Though in some cases it feels redundant; "notifications" is illustrated by the bell icon almost everywhere on the internet... I think aria-label should be used in that case, instead of a tooltip.


I'll work on a PR for this change, but it will be my first one on this repository. I probably won't get everything right in the first try.

@sstrubberg sstrubberg added proposal: accepted This request has gone through triaging and we are accepting PR's against it. needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. and removed proposal: needs more research 🕵️‍♀️ labels Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ui-shell needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. proposal: accepted This request has gone through triaging and we are accepting PR's against it. role: dev 🤖 type: a11y ♿ type: enhancement 💡
Projects
Status: 📋 Backlog
Status: Later 🧊
Development

No branches or pull requests

4 participants