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.

Sign up for GitHub

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

Jump to bottom

V8: Accessibility Changes For umbEditorHeader Directive #6315

Merged
merged 32 commits into from
Oct 30, 2019
Merged

V8: Accessibility Changes For umbEditorHeader Directive #6315

merged 32 commits into from
Oct 30, 2019

Conversation

RachBreeze
Copy link
Contributor

This PR applies the accessibility amends made in #6236 and #5760 to the umEditorHeader Directive.

Specifically it aims to:

  • Set the <title> attribute on the page based on the user's location
  • Allow screen reader users to understand when they are creating an item what item they are creating
  • Allow screen reader users to understand when they are editing an item what item they are editing

@RachBreeze
Copy link
Contributor Author

Hi @nul800sebastiaan, @poornimanayar and @emmaburstow
This is marked as a draft PR because of the amends I have made to a core directive specifically the addition of the "editorfor". It would be good to get this approach reviewed please. If it is the correct approach then there are a few more controllers I need to add this to, specifically those that are used in the settings section.
Thank you in advance for your time

@emmaburstow
Copy link
Contributor

Hi Rachel,

Thanks for getting in touch. We'll be sure to take a look and give you some feedback.

Emma

@RachBreeze
Copy link
Contributor Author

Hi Emma
Really appreciate that thank you

@nul800sebastiaan
Copy link
Member

Hi @RachBreeze, this looks fine to me, but I don't know why you're doubting this direction?

@RachBreeze RachBreeze mentioned this pull request Sep 16, 2019
Merged
1 task
@zersiax
Copy link
Contributor

zersiax commented Sep 17, 2019

This looks good to me, it will add some useful info. I think @RachBreeze is mainly being careful because the core code is touched, am I correct there?

@RachBreeze
Copy link
Contributor Author

Hi @nul800sebastiaan and @zersiax thank you for reviewing. Florian is correct I was concerned about touching something that everyone can reuse. I will look to finish off and submit a PR

Rachel Breeze and others added 19 commits October 2, 2019 19:45
Merge branch 'master' of https://github.com/RachBreeze/Umbraco-CMS in…
a52d36a 
…to v8/a11y_umbeditor_directive

# Conflicts:
#	src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js
Fixed error when creating a new user
8dbbd8c
This change makes sure that if a relationship type is edited, followe…
9066776 
…d by a create, the title of the page is reset, a seperate PR will address the a11y requirements for this controller
Umbraco Header directive now uses "create" rather than create item
ed9ec07
Made the ediorFor property more generic
9c1effe
Made the ediorFor property more generic
e312610 
Umbraco Header directive now uses "create" rather than create item
Edit and create template now aligns to the more a11y header directive
5c61fd7
Added support for a11y name and title on settings->scripts
88bfce3
Added support for a11y name and title on settings->stylesheets
faae644
Added support for a11y name and title on settings->partial views
d24933d
Added support for a11y name and title on settings->partial view macros
b0beccc
Merge branch 'v8/dev' of https://github.com/RachBreeze/Umbraco-CMS in…
063900f 
…to v8/a11y_umbeditor_directive

# Conflicts:
#	src/Umbraco.Web.UI.Client/src/views/relationtypes/create.controller.js
Added support for a11y name and title when editing members
fc741df
Added support for a11y name and title when editing member groups
d821265
Updated to ignore package-lock
67cb609
Added support for a11y name and title when editing media types
0fa2d24
Added the ability to prevent the title being set via the umbeditorhea…
d9d7878 
…der directive, currently for use when the content editor is editing media via a content page
No longer sets the page title by default
39faa87
No longer sets the page title by default
1e35b14
@RachBreeze RachBreeze marked this pull request as ready for review October 7, 2019 19:27
@RachBreeze
Copy link
Contributor Author

Hello,
This pull request is now ready for review. In order to preserve backward compatibility the page title is only set if the header is explicitly told to set it.
The header directive is used in a number of areas. This PR aims to address accessibility when it is used in the following locations

  • Create/ edit media
  • Create/ edit document types
  • Create/ edit media types
  • Create/ edit member types
  • Create/ edit content templates
  • Create/ edit templates
  • Create/ edit partial views
  • Create/ edit partial view macro files
  • Create/ edit style sheets
  • Create/ edit scripts

@RachBreeze
Copy link
Contributor Author

There are a couple of areas still to do (and I will submit PRs for them) but am looking forward to getting feedback. Cheers

Rachel Breeze and others added 7 commits October 21, 2019 17:25
Merge branch 'v8/dev' of https://github.com/RachBreeze/Umbraco-CMS in…
dbe71b7 
…to v8/a11y_umbeditor_directive

# Conflicts:
#	src/Umbraco.Web.UI.Client/package-lock.json
#	src/Umbraco.Web.UI/Umbraco/config/lang/en.xml
#	src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml
Fixing redering issues following refactor
1cb55a0
@emmaburstow
Don't ignore package-lock.json
a59f402
Merged languages
f971cb4
@emmaburstow
Merge branch 'v8/dev' into v8/a11y_umbeditor_directive
0f48100
@emmaburstow
Merge branch 'v8/dev' into pr/6315
29ce126 
# Conflicts:
#	src/Umbraco.Web.UI/Umbraco/config/lang/en.xml
#	src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml
@nul800sebastiaan
Fix up merge errors
29bb8e6
@emmaburstow emmaburstow merged commit 1bac814 into umbraco:v8/dev Oct 30, 2019
This was referenced Oct 30, 2019
Merged
Merged
Merged
nul800sebastiaan pushed a commit that referenced this pull request Oct 31, 2019
@nul800sebastiaan
Fix for #6315 where the loading banner continues to display if there …
ce08337 
…is no current editor state
@RachBreeze RachBreeze mentioned this pull request Nov 7, 2019
Merged
kjac pushed a commit that referenced this pull request Jan 14, 2020
@RachBreeze @kjac
Moved the accessibiity texts created in #6315 and #5760 into visually…
d841fdc 
…hidden section to comply with other commits (#6978)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
category/accessibility community/pr release/8.4.0 type/feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@RachBreeze @emmaburstow @nul800sebastiaan @zersiax @clausjensen @umbrabot