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

[4.0] Dialoges Should be labeled #34383

Merged
merged 11 commits into from Jul 21, 2021
Merged

[4.0] Dialoges Should be labeled #34383

merged 11 commits into from Jul 21, 2021

Conversation

Krshivam25
Copy link
Contributor

@Krshivam25 Krshivam25 commented Jun 3, 2021

For any container whose contents act as a dialog box (for example, a modal dialog asking the user to make a choice or respond to an action being taken), give it a descriptive label or name, so that assistive technology users can easily discover what its purpose is.

Testing Instructions :
Screenshot (269)

Arrow Indicating Dialog box .

Improvement: Dialog was not accessible before. Whenever we are adding a dialog role we must provide an accessible name for a dialog, which can be done with the aria-label or aria-labelledby attribute.

@chmst chmst added the a11y Accessibility label Jun 3, 2021
@ghost
Copy link

ghost commented Jun 4, 2021

@Krshivam25 can you please provide test instructions?

@jwaisner
Copy link
Member

jwaisner commented Jun 6, 2021

@Krshivam25 , can you please provide testing instructions for your PR?

@RickR2H
Copy link
Member

RickR2H commented Jun 17, 2021

I have tested this item ✅ successfully on a3f8154


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34383.

@RickR2H
Copy link
Member

RickR2H commented Jun 17, 2021

@jwaisner and @sandramay0905 Just look if the aria-label is added to the dialog after applying the patch.

screen shot 2021-06-17 at 11 46 47


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34383.

@brianteeman
Copy link
Contributor

Instead of duplicating the text it is better to add an id to the heading and use aria-labelledby

@Quy Quy added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Jul 2, 2021
@hans2103
Copy link
Contributor

hans2103 commented Jul 16, 2021

@Krshivam25

you write:

<joomla-alert type="info" dismiss="true" class="js-pstats-alert hidden" role="alertdialog" aria-label="<?php echo Text::_('PLG_SYSTEM_STATS_LABEL_MESSAGE_TITLE'); ?>">
	<div class="alert-heading"><?php echo Text::_('PLG_SYSTEM_STATS_LABEL_MESSAGE_TITLE'); ?></div>

and just like @brianteeman writes, you can prevent double text by another implementation...using aria-labelledby

<joomla-alert type="info" dismiss="true" class="js-pstats-alert hidden" role="alertdialog" aria-labelledby="alert-stats-heading">
	<div id="alert-stats-heading" class="alert-heading"><?php echo Text::_('PLG_SYSTEM_STATS_LABEL_MESSAGE_TITLE'); ?></div>

Copy link
Contributor

@hans2103 hans2103 left a comment

Choose a reason for hiding this comment

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

you have added:

<div id="alert-stats-heading" class="alert-heading"><?php echo Text::_('PLG_SYSTEM_STATS_LABEL_MESSAGE_TITLE'); ?></div>

please remove

<div class="alert-heading"><?php echo Text::_('PLG_SYSTEM_STATS_LABEL_MESSAGE_TITLE'); ?></div>

there is a duplicate alert-heading now

@Quy Quy removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Jul 17, 2021
@Quy
Copy link
Contributor

Quy commented Jul 17, 2021

I have tested this item ✅ successfully on bbde261


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34383.

1 similar comment
@hans2103
Copy link
Contributor

I have tested this item ✅ successfully on bbde261


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34383.

@Quy Quy removed the a11y Accessibility label Jul 18, 2021
@Quy
Copy link
Contributor

Quy commented Jul 18, 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34383.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 18, 2021
@Quy Quy added this to the Joomla 4.0 milestone Jul 18, 2021
@Krshivam25
Copy link
Contributor Author

Thank you

@Quy Quy added the a11y Accessibility label Jul 20, 2021
@chmst chmst merged commit a35cbca into joomla:4.0-dev Jul 21, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 21, 2021
@chmst
Copy link
Contributor

chmst commented Jul 21, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Accessibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants