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

update toast documentation on accessibility #27185

Merged
merged 3 commits into from
Sep 18, 2018

Conversation

Lausselloic
Copy link
Contributor

update documentation following discussion with @patrickhlauke on PR #27155

Maybe need a review for english because it's not my native langage.

Also add a question about default toast timeout actually defined to 500ms maybe most of users will let it as is by copy/paste... don't know what's the usual timer on toaster but maybe increase it by default at 3seconds or 5seconds?

@Johann-S
Copy link
Member

Johann-S commented Sep 6, 2018

About the timeout I chose that value 500ms because for small message it'll be enough and for long message it won't be enough.
So we have to help our users to think by themselves we are not their mother/father and they are developers.
You added a useful information about to choose the right timeout so that's enough 👍

Personnaly the only doubts I have, it's, is it useful to have a show delay ?

site/docs/4.1/components/toasts.md Outdated Show resolved Hide resolved
site/docs/4.1/components/toasts.md Outdated Show resolved Hide resolved
site/docs/4.1/components/toasts.md Outdated Show resolved Hide resolved
site/docs/4.1/components/toasts.md Outdated Show resolved Hide resolved
@XhmikosR
Copy link
Member

@mdo @patrickhlauke: LGTY?

@Johann-S
Copy link
Member

Personnaly the only doubts I have, it's, is it useful to have a show delay ?

What do you think about that @XhmikosR ?

@XhmikosR
Copy link
Member

@Johann-S: not sure to be honest, I'm not sure the branch is stable enough yet.

BTW I rebased this and the base branch to resolve the conflicts.

@Johann-S
Copy link
Member

@Lausselloic I updated the main PR by removing the show delay and fixing our documentation, can you rebase your branch ?

…e button everywhere just let it when autohide is set to false
@Johann-S
Copy link
Member

finally I did it 😆

mdo
mdo previously requested changes Sep 18, 2018
Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple text changes and then this seems good to go.


You also need to adapt the `role` and `aria-live` level depending on the content. If it's an important message like an error, use `role="alert" aria-live="assertive"`, otherwise use `role="status" aria-live="polite"` attributes.

Be carefull to adapt the [`delay` timeout](#options) depending on the content to give all users enough time to read the content.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed a typo here, but decided to rewrite this bit. Can you change this? If not, I can push a commit.

As the content you're displaying changes, be sure to update the [`delay` timeout](#options) to ensure people have enough time to read the toast.

<div role="alert" aria-live="assertive" aria-atomic="true">...</div>
</div>
{% endhighlight %}

If you use `autohide: false` then you need to add a `close` button into the header to enable users to dismiss that element.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to:

When using `autohide: false`, you must add a close button to allow users to dismiss the toast.

@XhmikosR XhmikosR merged commit ab43f27 into twbs:boom-toasted Sep 18, 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 this pull request may close these issues.

5 participants