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

Accessibility support #465

Open
dmurdoch opened this issue Mar 25, 2023 · 5 comments · May be fixed by #469
Open

Accessibility support #465

dmurdoch opened this issue Mar 25, 2023 · 5 comments · May be fixed by #469

Comments

@dmurdoch
Copy link

By default, htmlwidgets are wrapped in <div> tags, which don't support "alt" attributes for explanatory text. In yihui/knitr#2243 I have proposed that htmlwidgets which use the "aria-labelledby" attribute should get a text label added, which (at least some) screen readers will treat like alt text.

I can do this for the rgl widget by adding a widget_html.rglWebGL() function (https://github.com/dmurdoch/rgl/blob/635349d8c2c7deef5fcfa292340c93fb254a3286/R/rglwidget.R#L401-L404) but maybe this should be added by the htmlwidgets:::widget_html.default() function (

htmlwidgets/R/htmlwidgets.R

Lines 287 to 293 in e234c0e

widget_html.default <- function (name, package, id, style, class, inline = FALSE, ...) {
if (inline) {
tags$span(id = id, style = style, class = class)
} else {
tags$div(id = id, style = style, class = class)
}
}
), probably conditional on some setting in the widget.

@dmurdoch
Copy link
Author

The PR yihui/knitr#2243 has now been merged.

@dmurdoch dmurdoch linked a pull request May 24, 2023 that will close this issue
@cpsievert
Copy link
Collaborator

cpsievert commented Jun 20, 2023

@dmurdoch have you considered having {knitr} automatically add the aria-labelledby attribute to the widget's HTML (by extracting the id from the HTML and using that to populate the aria-labelledby attribute)? That seems like it could lead to a cleaner solution than #469 (it doesn't feel quite right for {htmlwidgets} to provide options for a {knitr} feature). The main thing I'm not sure of that might be an issue is whether the aria-labelledby attribute is ever not desirable when fig.cap is provided (but even in that case, it still feels like {knitr}'s responsibility to add an opt-out for that?)

@dmurdoch
Copy link
Author

@cpsievert : I hadn't considered that before. Now I have, I think it's a bad idea: currently knitr doesn't touch the code of an htmlwidget that it's including in a document. To do so, it would need to parse enough of the htmlwidget code to know that it's safe to modify it. I don't think knitr currently does anything like that.

As to the last part of your question: I can imagine a widget that provides an alternate label that is unrelated to fig.cap/fig.alt from knitr, and does it in a way that knitr wouldn't recognize. Then there'd be confusion.

So I think the current solution is better (though obviously there might be an even better one).

@cpsievert
Copy link
Collaborator

cpsievert commented Jun 20, 2023

To do so, it would need to parse enough of the htmlwidget code to know that it's safe to modify it. I don't think knitr currently does anything like that.

That's a fair point. Perhaps a workaround would be to get/set attributes via JS? Maybe by having knitr:::add_html_caption() append a <script> tag that looks something like this?

<script>
(function() {
  var el = document.getElementById({{id}}); // id would be populated in R via the widget's HTML
  var aria = el.getAttribute('aria-labelledby');
  if (!aria) {
     el.setAttribute('aria-labelledby', {{id}} + "-knitr-aria");
     el.parentElement.querySelector('.caption').setAttribute('id', {{id}} + "-knitr-aria");
  }
  document.currentScript.remove();
})();
</script>

@dmurdoch
Copy link
Author

The Javascript would need to be more complicated than that - the widget would need some way to communicate to knitr that it wants the label. That's what my htmlwidgets PR was doing with the aria-labelledby attribute. Some widgets shouldn't get aria labels -- see @gtritchie's first comment to the PR #469.

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 a pull request may close this issue.

2 participants