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

Change the order of the alert component template content to correct screen readers output #3171

Closed
peterblazejewicz opened this issue May 5, 2019 · 1 comment · Fixed by #3176

Comments

@peterblazejewicz
Copy link
Contributor

Bug description:

With the current content rendering order within alert component template:

    <button *ngIf="dismissible" type="button" class="close" aria-label="Close" i18n-aria-label="@@ngb.alert.close"
      (click)="closeHandler()">
      <span aria-hidden="true">&times;</span>
    </button>
    <ng-content></ng-content>

the content of the alert when read by screen reader always starts with Close, followed by the ng-template slot content. As the users of the ng-bootstrap cannot swap that rendering order (as opposed to BS users), this seems to impact the user experience greatly, while it can be easily fixed:

    <ng-content></ng-content>
    <button *ngIf="dismissible" type="button" class="close" aria-label="Close" i18n-aria-label="@@ngb.alert.close"
      (click)="closeHandler()">
      <span aria-hidden="true">&times;</span>
    </button>

Link to minimally-working StackBlitz that reproduces the issue:

https://ng-bootstrap.github.io/#/components/alert/examples

Versions of Angular, ng-bootstrap and Bootstrap:

Angular:
n/a
ng-bootstrap:
4.1.2
Bootstrap:
n/a

@benouat
Copy link
Member

benouat commented May 6, 2019

Hey @peterblazejewicz, thanks to report this issue.

That is totally valid. Even Bootstrap provides markup guideline rendering the close button after the content. (See here)

<div class="alert alert-warning alert-dismissible fade show" role="alert">
  <strong>Holy guacamole!</strong> You should check in on some of those fields below.
  <button type="button" class="close" data-dismiss="alert" aria-label="Close">
    <span aria-hidden="true">&times;</span>
  </button>
</div>

@maxokorokov maxokorokov added this to the 4.1.3 milestone May 6, 2019
maxokorokov added a commit to maxokorokov/ng-bootstrap that referenced this issue May 6, 2019
benouat pushed a commit that referenced this issue May 7, 2019
Order between the close button and `<ng-content>` used to be wrong. Content needs to render first in order for a11y/screen-reader to properly read it before the close button.

Fixes #3171
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.

3 participants