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

V8: Accessibility improvements for page name entry #5585

Closed
wants to merge 8 commits into from
Closed

V8: Accessibility improvements for page name entry #5585

wants to merge 8 commits into from

Conversation

RachBreeze
Copy link
Contributor

This PR aims to fix 43, 69, 75, 81 from the from the A & AA accessibility issue list #5277

Creating a content item
When creating a new content type there is screen reader only text which says:
"Create" + content type name

The concept is that it create wors as per Microsoft Teams, here when a user creates a folder they see:
image

Editing a content item
When editing an existing content item there is screen reader only text which says
"Edit" + content name

Name label
The "name" text box also now now has a hidden label which is "content type name" + Name

Placeholder
The placeholder in the name textbox remains the same as prior to this change, as placeholder text provides guidance for many users, placeholder text is not a replacement for labels.

Class amends
This PR contains a sr-only class for displaying content for screen readers only. However there is nothing to stop the hidden text being displayed on the screen if HQ deem fit (subject to improved styling)

*Language amends
The language files now have accessibility specific place holders these have been added to en.xml and en_us.xml only

@MMasey
Copy link
Contributor

MMasey commented Jun 5, 2019

Awesome work @RachBreeze. In regards to the .sr-only class, I've also create a .visually-hidden class in a separate PR https://github.com/umbraco/Umbraco-CMS/pull/5544/files. Just leaving a note here so that we can refactor them into one class when after they have both been merged in 😄

@RachBreeze
Copy link
Contributor Author

@MMasey oops missed that sorry, more than happy to tidy up either now or when they're merged.

@MMasey
Copy link
Contributor

MMasey commented Jun 6, 2019

@RachBreeze I think it's fine for now. Let's keep an eye on both of them and once the are both merged in we can all agree on a preferred solution and update accordingly 🙂

@nul800sebastiaan, @emmaburstow, @poornimanayar, @dawoe or @abjerner, could you add the accessibility label to this please. Thanks ❤️

@nul800sebastiaan
Copy link
Member

Hi @RachBreeze - I think you accidentally mixed a lot of things up here. I was going to try to fix it but it also seems you deleted your fork?

Just a tip: you don't need to delete forks when you're done, 2 things to keep in mind:

  1. Each PR you do should be a new branch made from v8/dev - that way you can work on and potentially update a PR at the same time as working on and updating another PR (by switching between branches).
  2. When you switch back to dev/v8 you can update it from the central repository (the Umbraco HQ one) - this is described here: https://github.com/umbraco/Umbraco-CMS/blob/v8/dev/.github/CONTRIBUTING.md#keeping-your-umbraco-fork-in-sync-with-the-main-repository

Additionally, once you've synced v8/dev with the upstream (see link above) you could then switch to your PR branch and merge v8/dev into it, so you get the latest changes from us into your PR as well if needed.

I will close this PR for now, can I kindly ask you to resubmit it so that the work is not as completely tangled up as it is now?

Other than that I think it looks fine, well done!

@RachBreeze
Copy link
Contributor Author

RachBreeze commented Jun 25, 2019

Cheers @nul800sebastiaan got slightly lost with the PR process here so thank you for your comments (I've done the same on my other outstanding PR. I will fix both)

@nul800sebastiaan
Copy link
Member

No worries, it's confusing when you first get started, happy to help! Let me know if you have any questions.

@RachBreeze
Copy link
Contributor Author

RachBreeze commented Jul 5, 2019

Hi @nul800sebastiaan just to let you know I've resubmitted this as #5760 cheers.

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

Successfully merging this pull request may close these issues.

3 participants