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

Accessible icons #8

Closed
mine-cetinkaya-rundel opened this issue Jul 16, 2022 · 16 comments
Closed

Accessible icons #8

mine-cetinkaya-rundel opened this issue Jul 16, 2022 · 16 comments

Comments

@mine-cetinkaya-rundel
Copy link
Contributor

There are some recommendations at https://fontawesome.com/docs/web/dig-deeper/accessibility for accessibility of Font Awesome icons. Generally, they include adding aria-hidden="true" and adding a title for icons with semantic or iterative meaning. It would be good to incorporate these in the extension.

@mcanouil
Copy link
Contributor

mcanouil commented Jul 16, 2022

This can easily (except for the title attribute) be done by simply adding the code in

return pandoc.RawInline('html', "<i class=\"fa-" .. group .. " fa-" .. icon .. "\"></i>")

@mcanouil
Copy link
Contributor

I added this feature in #10
image

@mcanouil
Copy link
Contributor

mcanouil commented Jul 22, 2022

Should the aria-label be used instead of relying on "Auto-Accessibility with Kits" from FA? 🤔
I will try a screen reader to check if the current implementation (#10) does what it is supposed to.

@cscheid
Copy link
Contributor

cscheid commented Jul 24, 2022

I will defer to @jooyoungseo's expertise on this. JooYoung, what is the right way for us to address accessibility concerns in these icon packages?

@mcanouil
Copy link
Contributor

mcanouil commented Jul 24, 2022

@jooyoungseo
Copy link

jooyoungseo commented Jul 25, 2022

I see some issues with fontawesome implementation in {shiny} and {fontawesome} packages. I will further investigate this issue. Using title attribute is often debatable like this post. Also, some screen readers do not recognize title attribute unless users explicitly turn on related settings.

@jooyoungseo
Copy link

I think fontawesome's a11y recoomendation is debatable. they recommend using aria-hidden for the icons and provide alt text on a parent-level via aria-label. However, in many cases, fontawesome icons are used not just for decorative purpose, but also for informative communication. I think providing aria-label within <i> tag is more robust solution, and I would assign aria-label == fontname. I won't recommend aria-hidden.

@jooyoungseo
Copy link

In the following example (retrieved from example(checkboxGroupInput, package = "shiny")), icons are used for informative purpose. However, the checkbox is not accessible for screen readers until explicit aria-label is used by content creator because this does not provide any icon alt text by default. It would be more desirable and safer if the aria-label is provided by default equal to font name and allow content creators to override that value if necessary.

library(shiny)

ui <- fluidPage(
  checkboxGroupInput("icons", "Choose icons:",
    choiceNames =
      list(icon("calendar"), icon("bed"),
           icon("cog"), icon("bug")),
    choiceValues =
      list("calendar", "bed", "cog", "bug")
  ),
  textOutput("txt")
)


server <- function(input, output, session) {
  output$txt <- renderText({
    icons <- paste(input$icons, collapse = ", ")
    paste("You chose", icons)
  })
}

shinyApp(ui, server)

@mcanouil
Copy link
Contributor

mcanouil commented Jul 25, 2022

@jooyoungseo Thanks for these comments.

To note, screen readers are not the target tools for title, it's mostly for mouse pointers readers.
Meaning, currently (with #10 merged) semantic icons are invisible to screen readers as is.

If I summarise you suggest to:

  • never include aria-hidden
  • add a aria-label parameter in the shortcode to allow this to define within the i HTML tag, with a default value being the FA icon name

Did I miss anything?

@cscheid I can work on a PR if that's ok with you?

@mcanouil
Copy link
Contributor

mcanouil commented Jul 25, 2022

Hum, in my first tests (Draft PR #12), MacOS screen reader seems to be unable to read the aria-label from the example below:
Screenshot showing on the left a rendered document including inline icon as "A text with a {{< fa folder >}} icon in it." and on the right html code from developer mode "<p>A text with a "<i class=\"fa-solid fa-folder\" aria-label=\"folder\"></i> icon in it.</p>"

@mcanouil mcanouil mentioned this issue Jul 25, 2022
@jooyoungseo
Copy link

@mcanouil -- Would you mind providing me with the example code in plain text? It's currently displayed in image.

@mcanouil
Copy link
Contributor

The following quarto/markdown:

A text with a {{< fa folder >}} icon in it.

produces the following HTML where MacOS screen reader omit the icon when reading:

<p>A text with a <i class="fa-solid fa-folder" aria-label="folder"></i> icon in it.</p>

This is the results of wip #12

@cscheid
Copy link
Contributor

cscheid commented Oct 20, 2022

I think we have this now, so I'm going to close it.

@cscheid cscheid closed this as completed Oct 20, 2022
@dfolio
Copy link

dfolio commented Apr 7, 2023

I think accessibility is great.
But, when I'm running Chrome Lighthouse Audits on a page using this fontawesome extension (for instance with {{< fa file-pdf >}}), it complains about aria roles and attributes and I get:

Accessibility: [aria-*] attributes do not match their roles

Each ARIA role supports a specific subset of aria-* attributes. Mismatching these invalidates the aria-* attributes.

Failing Elements: i.fa-solid.fa-file-pdf

with the following html code

<i class="fa-solid fa-file-pdf" aria-label="file-pdf"></i>

@mcanouil
Copy link
Contributor

mcanouil commented Apr 7, 2023

I am not sure what it means exactly ... 🤔

Note that the aria-label here is the default that takes the icon label.

@jooyoungseo
Copy link

jooyoungseo commented Apr 7, 2023

FYI, automatic accessibility audit tools are only able to capture ~80% violation, and sometimes gives us an aggressive result. I don't think we have used any aria role particularly for fontawesome and the warning above can be disregarded. I used role="presentation" for icons when I worked with shiny team, but that's deprecated. I will take a close look at the fontawesome a11y this summer.

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

No branches or pull requests

5 participants