- Issue created by @Chi
- Status changed to Needs review
over 1 year ago 3:19am 27 March 2023 - 🇮🇳India gauravvvv Delhi, India
I have removed the
role="button". attached patch for same. - First commit to issue fork.
- Assigned to Guru2023
- Issue was unassigned.
- 🇮🇳India Guru2023
Verified the patch #2 and reviewed it. The patch works fine and I have added the before and after screenshots for reference.
ready for RTBC. - @rckstr_rohan opened merge request.
- 🇺🇸United States smustgrave
Agree it should be removed there but wonder if it should be added to the summary as that does accordion functionality.
- 🇮🇳India Akshay kashyap
@Gauravvvv Thanks for the Work. Applied patch #2 cleanly. Removed role=button on a status report page.
- First commit to issue fork.
- 🇮🇳India gauravvvv Delhi, India
Added role=button to summary as that does accordion functionality.
- 🇮🇳India Akshay kashyap
Verified patch #11 and reviewed it. The patch works fine and I have added the screenshots for reference.
ready for RTBC. - Status changed to RTBC
over 1 year ago 5:08am 29 March 2023 - 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Removing credit for creating an MR when there was already a patch.
Removing credit for rebasing the MR - Status changed to Needs work
over 1 year ago 11:25pm 3 April 2023 - 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
According to https://developer.mozilla.org/en-US/docs/Web/HTML/Element/summary#techni... there are no permitted roles for a summary element and button is implied, so this change is not correct
+++ b/core/themes/claro/templates/status-report-grouped.html.twig @@ -22,7 +22,7 @@ - <summary id="{{ group.type }}" class="claro-details__summary claro-details__summary--system-status-report">{{ group.title }}</summary> + <summary id="{{ group.type }}" class="claro-details__summary claro-details__summary--system-status-report" role="button">{{ group.title }}</summary> - Status changed to Needs review
over 1 year ago 2:34am 4 April 2023 - 🇮🇳India gauravvvv Delhi, India
Addressed #15, attached interdiff for same. please review
The last submitted patch, 16: 3350384-16.patch, failed testing. View results →
- Status changed to Needs work
over 1 year ago 4:12am 4 April 2023 - 🇷🇺Russia Chi
Re #15 Drupal always applies
role="button"to summary elements. See #1848684: Add ARIA role to DETAILS → . However, it's important that along withrole="button"the summary element should get a certain combination of aria attributes. Namely,aria-controls,aria-expanded,aria-pressed. Those are needed to make the details/summary element behave like a disclosure widget. Otherwise the summary would become a simple button and screen readers will not announce the state of the element. Which would make thins much worse.Here are some details.
https://www.scottohara.me/blog/2022/09/12/details-summary.html#consisten...Given that #1848684: Add ARIA role to DETAILS → was discussed ten years ago we might need to rethink the whole approach. This might be no longer needed at all.
