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

.selected selectors should always include [aria-selected=true] #790

Closed
shawnbot opened this issue May 9, 2019 · 0 comments
Closed

.selected selectors should always include [aria-selected=true] #790

shawnbot opened this issue May 9, 2019 · 0 comments
Labels
minor release ⭐️rep an industry leading reputation

Comments

@shawnbot
Copy link
Contributor

shawnbot commented May 9, 2019

We have a ton of styles that target selected elements with .selected. In order for Primer to support accessibility improvement efforts, we should always apply the same styles to [aria-selected=true], as we've done in #786.

Here are the ways that we could go about it:

  1. Just rewrite every instance of &.selected to &[aria-selected=true], &.selected. Easy peazy.

  2. Add Sass variables that represent selector "qualifiers" for use with selector functions:

    $selected-selector: "[aria-selected=true], .selected";
    
    .SomeComponent {
      #{selector-append(&, $selected-selector)} { /* ... */ }
    }

    Full disclosure: I haven't used these before so I don't know how they work, and this syntax is pretty gnarly.

  3. Add a Sass function, mixin, or postcss plugin that would support a more terse form:

    .SomeComponent {
      // again, this syntax is ugly
      #{selected(&)} { /* ... */ }
    
      // this is nicer...
      @include selected(&) { /* ... */ }
    
      // or maybe use a postcss plugin for :matches()?
      &:matches($selected-selector) { /* ... */ }
    }

After writing these out I'm in favor of the first option, particularly because it lines up with our stated principles. But I do think there's value in looking at the alternatives! 😁

I'm marking this as a minor release issue because technically it would be a new "feature"; however, we might want to consider this a general accessibility issue that we're "fixing".

@shawnbot shawnbot added minor release ⭐️rep an industry leading reputation a11y labels May 9, 2019
flavianunes pushed a commit to flavianunes/css that referenced this issue Nov 12, 2019
flavianunes pushed a commit to flavianunes/css that referenced this issue Dec 26, 2019
flavianunes pushed a commit to flavianunes/css that referenced this issue Dec 26, 2019
@simurai simurai closed this as completed Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor release ⭐️rep an industry leading reputation
Projects
None yet
Development

No branches or pull requests

2 participants