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

Modal - Missing aria-labelledby on modal container #1477

Closed
giampierobono opened this issue Apr 11, 2017 · 5 comments · Fixed by #2049
Closed

Modal - Missing aria-labelledby on modal container #1477

giampierobono opened this issue Apr 11, 2017 · 5 comments · Fixed by #2049

Comments

@giampierobono
Copy link
Contributor

giampierobono commented Apr 11, 2017

To improve the modals accessibility, it could be useful to add the aria-labelledby attribute on the modal container (the element having the role=dialog set).

This attribute will help SR users in having a clear "description" about the modal content and should be linked to the modal header (normally the h tag).

According to W3C specifications(here: https://cdn.rawgit.com/w3c/aria-practices/master/aria-practices.html#dialog_modal) this attribute seems to be mandatory.

@dstanich
Copy link

We're looking to migrate to ng-bootstrap and also came across this. It would be nice if it could be implemented in the same way that the AngularJS version was done, as a config value passed in when opening the dialog:

ui.bootstrap doc: https://angular-ui.github.io/bootstrap/#!#modal

@pkozlowski-opensource
Copy link
Member

I was going over a proposed PR #2049 and before we merge it I would like to explore another, possible APIs. I want to brainstorm on it as I've got some open questions to the API proposed in the mentioned PR.

Alternative proposal

Let's add the new ariaId (or just id) option to NgbModalOptions. On top of this we would expose this option to a modal's window content template like so:

<ng-template #content let-c="close" let-d="dismiss" let-id="ariaId">
  <div class="modal-header">
    <h4 class="modal-title" id="{{ariaId}}">Modal title</h4>
    <button type="button" class="close" aria-label="Close" (click)="d('Cross click')">
      <span aria-hidden="true">&times;</span>
    </button>
  </div>
  <div class="modal-body">
    <p>One fine body&hellip;</p>
  </div>
  <div class="modal-footer">
    <button type="button" class="btn btn-outline-dark" (click)="c('Close click')">Close</button>
  </div>
</ng-template>

This way a user can specify title id only once (when opening a modal) and not repeat it in both a template and TS code. Moreover we could generate this id automatically if not provided (as we do for other controls).

@ymeine
Copy link
Contributor

ymeine commented Jan 25, 2018

I agree we could improve things regarding the handling of the id, to avoid repeating it or having to specify it at all.

However, there are several points:

  • the user will still have to tell how he wants this id to be used: is it for aria-labelledby or aria-describedby? Also, I guess both could coexist, so we would still need to handle both (both with a corresponding optional input property, a generated id if needed, etc.)
  • the id could be linked to an external element (in which case it's fine, we pass it when opening the Modal, and we simply don't use it in its template)
  • the user might still want to pass a label as a string (therefore using aria-label)

Regarding the API as proposed in #2049, I think we should make clear what the properties are doing and name them as close as possible to the name of the attributes they generate. I don't see any other way to support those 3 properties with enough flexibility without providing at least 3 inputs.

So, one could pass any of:

{
    ariaLabel: 'my label as a string', // generates 'aria-label'
    ariaLabelledBy: 'my-external-id', // still available in Modal's template, generates 'aria-labelledby'
    aria-describedby: true // using an auto-generated id, available in Modal's template, generates 'aria-describedby'
}

when opening the modal. What changes here is:

  • passing true uses an auto-generated id
  • ids are available in Modal's template

(remark: we still need to tell if we want an aria-labelledby, or an aria-describedby, or both, so no way to use simply an automatically generated id injected in the template)

@pkozlowski-opensource
Copy link
Member

OK, I hear what you are saying. I'm still not sold on the proposed API but I can't think of a better solution. Given that we don't have "perfect" solution let's do minimal required here so we don't commit to one direction and keep doors open.

So, let's add the aria-labelledby option, without any auto-generation nor propagation to content's template.

@gowthamSelvaraj
Copy link

any update on this?

@maxokorokov maxokorokov modified the milestones: 2.1.1, 2.2.0 Jun 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants