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

Fixed Group Headings in Content #12702

Merged
merged 6 commits into from
Jul 21, 2022
Merged

Fixed Group Headings in Content #12702

merged 6 commits into from
Jul 21, 2022

Conversation

tiffy74
Copy link
Contributor

@tiffy74 tiffy74 commented Jul 15, 2022

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes umbraco/Umbraco-CMS.Accessibility.Issues#44

Description

When a group is created on a document type - in the content view the name of the group appears at the top of the group.

image

This is styled to appear as a heading - but it has no correspending H tag or role=heading in the html - this means it does not get announced as a heading to assistive technology. The impact is that those unable to visually perceive the text will be unaware that they are in a group entitled 'Content'. This is the same for any new group created in the same way:

image

Therefore I have submitted a PR that fixes this issue by adding a H2 tag to the group header. H2 because if the associated PR for issue 42 is accepted then it makes sense that the group headings are the next level after the H1 that should be the overall heading for the page (eg content)

image

Further I have attached additional styling to the section header to match it's previous style so there is minimal impact upon how the groups look visually.

In order to do this - the CSS properties need to be removed from the parent element and placed upon the H2 within. Futher the original CSS also had an absolute value for the font-size. This has now been amended to use the same .Less variable that is used elsewhere on the site.

.umb-group-panel__header {
padding: 12px 20px;
display: flex;
align-items: center;
justify-content: space-between;
color: @grayDarker;
border-bottom: 1px solid @gray-9;
}

.umb-group-panel__header h2 {
font-size: @fontSizeMedium;
font-weight: bold;
}

Testing

In order to test this:

  • Create a dummy site with a basic document type to represent a content or home page
  • Add some groups such as 'Content' and 'Header' as per images above
  • Go to the content editing section of the backoffice
  • Create a new page and use whichever browser's 'inspect' tool over the content groups. These should now be H2
  • This also works correctly with a screen reader: note - in order for the headings to be nested correctly this PR should only be merged after issue 42

Corey Philipp and others added 5 commits April 30, 2022 01:56
Previous optimisation incorrectly filtered the index fields needed to return published results
Ensuring that MemberManager.ConfirmEmailAsync persists, same changes in pull request #12640 but for v9
@github-actions
Copy link

github-actions bot commented Jul 15, 2022

Hi there @tiffy74, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

  • It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
  • The automated tests all pass (see "Checks" tab on this PR)
  • The level of security for this contribution is the same or improved
  • The level of performance for this contribution is the same or improved
  • Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
  • If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
  • 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

# Conflicts:
#	src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs
#	src/Umbraco.Infrastructure/PublishedContentQuery.cs
#	src/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgery.cs
@nul800sebastiaan nul800sebastiaan changed the base branch from v9/contrib to v10/contrib July 19, 2022 12:13
@mikecp
Copy link
Contributor

mikecp commented Jul 21, 2022

Hi @tiffy74 ,

Thanks for this other accessibility PR! I merged the one linked to issue 42 just before this one, so it's looking all fine!

Cheers!

@bjarnef
Copy link
Contributor

bjarnef commented Aug 1, 2022

I think after changing the group headings to actually heading-element, it is taking too much vertical space.

image

Previous it looked something like this:

image

When using e.g. a laptop I think the group header is taking too much valuable space especially with many groups.

It also seems it has some odd styling now as the elements have changed.

image

where the tree group headers doesn't have same sizes.

image

@tiffy74
Copy link
Contributor Author

tiffy74 commented Aug 1, 2022

Yes I agree. I noticed this at the weekend. I have created a new issue on our accessibility issues log and will resolve this asap

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.

Section text - Text that looks like a heading must be coded as a heading
7 participants