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

[5.2] Small fix for the dt- tag in content details list #43729

Merged
merged 5 commits into from
Aug 8, 2024

Conversation

chmst
Copy link
Contributor

@chmst chmst commented Jul 1, 2024

Pull Request for Issue # .

Summary of Changes

The article info in articles is built as a definition list.
The definion term is the text for Details,
The definition description is a list of different items like author, publishing-date ad so on

If the user decides to hide the definition term then the dt-tag remains empty.
With this small PR the the details info is not empty but hides the text only for sighted users, screen readers still have the complete definition term.

This PR is of type "better than nothing"
because a more correct solution for the would be a definition list which writes for every element
<dt>Author</dt><dd>Mr. John Doe</dd>

Testing Instructions

Easiest test is any blog in joomla.
Just set some detail info in the corresponding menu item to "show"
Then show and hide the details
No change in frontend for sighted users, but check the source code.

Actual result BEFORE applying this Pull Request

If the Article Info Title is set on "hide". The dt is empty
grafik

Expected result AFTER applying this Pull Request

grafik

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Co-authored-by: Quy <quy@nomonkeybiz.com>
@brianteeman
Copy link
Contributor

because a more correct solution for the would be a definition list which writes for every element

Why not fix it this way

@chmst
Copy link
Contributor Author

chmst commented Jul 1, 2024

Can do. Do you think it is worth? It is not a big issue ... just somthing for accuracy fanatics like us

@brianteeman
Copy link
Contributor

If a job is worth doing it is worth doing well.

Assuming of course that it is the correct thing to do and wcag seems to suggest that it is the correct way and even without hiding the "Details" it is wrong.

https://www.w3.org/WAI/tutorials/page-structure/content/#description-lists

@chmst
Copy link
Contributor Author

chmst commented Jul 1, 2024

I like this quote. So I make this a draft for now.

@brianteeman
Copy link
Contributor

I like this quote. So I make this a draft for now.

its a very famous "english" quote. I am sure there will be a german equivalent

@chmst chmst marked this pull request as draft July 1, 2024 21:14
@chmst
Copy link
Contributor Author

chmst commented Jul 5, 2024

Reviewing the code for this details block I think it is better to leave as is and just add this PR.
What we have is not wrong, it is one definiton term with multiple descriptions and in in this context it is ok. I will not change other layouts.

@chmst chmst marked this pull request as ready for review July 5, 2024 18:11
@chmst chmst mentioned this pull request Jul 7, 2024
4 tasks
@dautrich
Copy link

I have tested this item ✅ successfully on e11afa7


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

1 similar comment
@fgsw
Copy link

fgsw commented Jul 29, 2024

I have tested this item ✅ successfully on e11afa7


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

@Quy
Copy link
Contributor

Quy commented Jul 29, 2024

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 29, 2024
@bembelimen bembelimen changed the base branch from 5.1-dev to 5.2-dev August 4, 2024 13:29
@bembelimen bembelimen changed the title [5.1] Small fix for the dt- tag in content details list [5.2] Small fix for the dt- tag in content details list Aug 4, 2024
@bembelimen
Copy link
Contributor

I've moved this to 5.2, I agree it's a bug fix, but what could happen is, that suddenly now with templates not supporting the CSS class, the label could appear, although it's disabled.

@pe7er pe7er self-assigned this Aug 8, 2024
@pe7er pe7er enabled auto-merge (squash) August 8, 2024 20:51
@pe7er pe7er merged commit a9980fe into joomla:5.2-dev Aug 8, 2024
3 of 4 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 8, 2024
@Quy Quy added this to the Joomla! 5.2.0 milestone Aug 8, 2024
@pe7er
Copy link
Contributor

pe7er commented Aug 9, 2024

Thanks @chmst !

@chmst chmst deleted the a11y-fix-detail-info-block branch September 9, 2024 12:52
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.

8 participants