Opened 13 months ago
Last modified 6 weeks ago
#145023 reviewing theme
THEME: pally – 1.3
Reported by: |
|
Owned by: |
|
---|---|---|---|
Priority: | previously reviewed | Keywords: | theme-pally accessibility-ready |
Cc: | a11y@… |
Description
pally - 1.2
An accessible & inclusive WordPress Theme, pally.
Theme URL - https://www.barrierefreiesblog.de/
Author URL - https://www.points.de/
Trac Browser - https://themes.trac.wordpress.org/browser/pally/1.2
WordPress.org - https://wordpress.org/themes/pally/
SVN - https://themes.svn.wordpress.org/pally/1.2
ZIP - https://wordpress.org/themes/download/pally.1.2.zip?nostats=1
Diff with previous version: [196403] https://themes.trac.wordpress.org/changeset?old_path=pally/1.1&new_path=pally/1.2
History:
Ticket | Summary | Status | Resolution | Owner |
---|---|---|---|---|
#144395 | THEME: pally – 1.1 | closed | not-approved | |
#145023 | THEME: pally – 1.3 | reviewing | ||
- RECOMMENDED: No reference to register_block_pattern was found in the theme. Theme authors are encouraged to implement custom block patterns as a transition to block themes.
- RECOMMENDED: No reference to register_block_style was found in the theme. Theme authors are encouraged to implement new block styles as a transition to block themes.
- RECOMMENDED: No reference to add_editor_style() was found in the theme. It is recommended that the theme implement editor styling, so as to make the editor content match the resulting post output in the theme, for a better user experience.
- RECOMMENDED No reference to the_post_thumbnail() was found in the theme. It is recommended that the theme implement this functionality instead of using custom fields for thumbnails.
- RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
Attachments (2)
Change History (22)
#3
@
13 months ago
@pointsgmbh I have tested your theme for the accessibility-ready-tag. It is not complete yet, as I could not test the menu with submenus. Also other points I have not tested yet, as it is manuell work. I can see, that you considered accessibillity and I'm really happy about it.
If you could fix the following issues, I will tested again, together with the menu and the other open points. If you have any questions, you can ask me here or on slack. I think you are from Stuttgart? If you want, you can also ping me in german.
The first issues I have found are the following.
## Error Report
Landmarks:
- “Skip to main” and “Go to Top” are defined as <nav> but those links are not navigations. Use a link instead
- Landmark banner <header> is not presend
- main navigation is there, but has too many elements in it (Logo, Subline, search). This should ne <header> and main navigation should only be navigation
- there is a landmark involved, that I don’t understand the purpose of: <nav aria-label="Accessibility helper" id="a11yhelper"></nav>. I Neither see a navigation nor I understand “accessibility helper” as this is too generic. Also my screen-reader says “empty object”
- 404: search in main has no landmark. in case someone removes the search in <aside> it is more difficult for screen-reader-users to find the search
- some for empty search results
- search in header has no landmark
Keyboard-Navigation
- “jump to main” the visibility part of the site goes to main, but the focus while using keyboard does not jump to main. It stays in the logo in “header”. It only changed the focus for screen-reader-users
- focused elements are not completely visible in focus.
- mobile view - focus for the menu button is not visible enough
Headlines
- static landing page. H1 is there, but visuell hidden
Color Contrast
- code block color does not match WCAG AA
Voice Over:
- not sure why this happens, but my screen-reader reads it our in german, instead of english. Usually if the site is defined with the language tag, it will use this language and all my testing tools don’t show any failure. The site is defined for me as US English.
Code
- positive tabindex is present in assets→ js → main.js row 8401
- links should not open in a new tab. If target_blank is insert you should notify the user about that. target blanks are insert in:
- Appearence → pally setup help → [Accessibility Blog by points](https://www.barrierefreiesblog.de/)
- Appearence → pally setup help → WP Accessibility
- a link for Accessibility Statement Generator - I could find in the Dashboard, but its in the code
- 4 more strings in the de_DE.po file
#4
@
13 months ago
Hallo @Travel_girl, danke für dein Feedback. Ich schaue mir die Punkte an und melde mich wieder! Gruß, Kerstin
#5
@
13 months ago
@pointsgmbh you are welcome. The public converation like here in tickets should stay in english, as kafleg is also working on this ticket and he does not speak german.
So for your understanding @kafleg here is the translation:
Hello @Travel_girl, thanks for your feedback. I'll look at the points and get back to you! Greetings, Kerstin
#8
@
12 months ago
Hi @Travel_girl,
I corrected most of the things you stated above, but I have some remaining questions before I re-submit:
1) Keyboard-Navigation
“jump to main” the visibility part of the site goes to main, but the focus while using keyboard does not jump to main. It stays in the logo in “header”. It only changed the focus for screen-reader-usersI
-> How do I test this? When I try it, the focus jumps to #primary, both visually and with the keyboard focus? From there it jumps to the next link. Maybe it's not working in an empty installation?
2) Keyboard-Navigation
focused elements are not completely visible in focus
-> Which ones do you refer to? Do you have an example? I couldn't find any...
Thank you!
Kerstin
#9
@
12 months ago
@kemai
1)I have checked this again and the focus changing to main actually works. Sorry, it was a bug somewhere on my end.
you can test it by using the tab key and using enter to active the Link.
2) For example the comment form, read more links, category links.
See Screenhots attached.
Thank you,
Maja
#10
@
12 months ago
Hi @Travel_girl, thanks for the explanation. I think this may be a Firefox issue, as it looks ok in the other browsers, they allow more space when tabbing. I checked with a site that has a great tab focus appearance, and it has the same problem in Firefox. You can test it by tabbing through the page here:
https://www.gov.uk/guidance/cost-of-living-payment
As soon as you reach the link below "Contents", the tab focus is also not fully visible. But you can correct that with the down arrow to see it better.
#11
@
12 months ago
- Summary changed from THEME: pally – 1.2 to THEME: pally – 1.3
pally - 1.3
An accessible & inclusive WordPress Theme, pally.
Theme URL - https://www.barrierefreiesblog.de/
Author URL - https://www.points.de/
Trac Browser - https://themes.trac.wordpress.org/browser/pally/1.3
WordPress.org - https://wordpress.org/themes/pally/
SVN - https://themes.svn.wordpress.org/pally/1.3
ZIP - https://wordpress.org/themes/download/pally.1.3.zip?nostats=1
Diff with previous version: [198733] https://themes.trac.wordpress.org/changeset?old_path=pally/1.2&new_path=pally/1.3
History:
Ticket | Summary | Status | Resolution | Owner |
---|---|---|---|---|
#144395 | THEME: pally – 1.1 | closed | not-approved | |
#145023 | THEME: pally – 1.3 | reviewing | ||
- RECOMMENDED: No reference to register_block_pattern was found in the theme. Theme authors are encouraged to implement custom block patterns as a transition to block themes.
- RECOMMENDED: No reference to register_block_style was found in the theme. Theme authors are encouraged to implement new block styles as a transition to block themes.
- RECOMMENDED: No reference to add_editor_style() was found in the theme. It is recommended that the theme implement editor styling, so as to make the editor content match the resulting post output in the theme, for a better user experience.
- RECOMMENDED No reference to the_post_thumbnail() was found in the theme. It is recommended that the theme implement this functionality instead of using custom fields for thumbnails.
- RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
#12
@
12 months ago
@Travel_girl
I re-submitted the theme. I have some remaining things / explanation:
- <nav aria-label="Accessibility helper" id="a11yhelper"></nav> -> this was/is a landmark for the plugin WP Accessibility. It will only show now if the plugin is installed and used.
- Screenreader language: I tested again, and changing the language of the screenreader only really works if you adjust your system to another language. I guess that screenreader users will know how to do it. I can't do more than set the the right lang-attribute in the code.
- positive tabindex is present in assets→ js → main.js row 8401 -> This is from jQuery (we are using 3.6). Can't do anything about this
The rest should be ok now?
Thanks, and greets!
Kerstin
#13
@
10 months ago
Hi @kafleg and @Travel_girl,
I just wanted to know if you are still reviewing the theme?
Best regards,
Kerstin
#14
@
10 months ago
@pointsgmbh Hey Kerstin, I have not noticed your last message, so sorry.
I will need to do a full a11y review of you theme, which will take me some time. I will get back to you, when I have the result.
Thanks,
Maja
#18
@
8 months ago
@kafleg @kemai still on my bucket list. Currently I have a lot on my plate regarding work. I will write an update, when I could manage to make an full Accessibility Test for the Theme.
#19
@
6 weeks ago
Hi!
I've tested the new version (1.3) and found a couple of things I would like to flag:
Menu position fixed + Skip to Content button
When we stick the menu at the top (customizer option), the content isn't 100% visible when using "Skip to Content" button. The title of the first post is hidden (we might add some top margin?).
Listing Posts - screen-reader-text
Currently, the "read more" button is populated with the post name in the screen-reader-text. example:
<a class="read-more" href="http://localhost:10068/hello-world/"> Read more <span class="screen-reader-text">Hello world!</span></a>
I'm wondering if adding a little more context would be more accessible, like "Read more about Hello World". example:
<a class="read-more" href="http://localhost:10068/hello-world/"> Read more about<span class="screen-reader-text">Hello world!</span></a>
Menu Navigation - screen-reader-text
Currently, the screen reader says: Link Navigation: Hello world
This sounds redundant, I recommend we remove "Navigation:" from the menu nav.
Dropdown Navigation - aria-label
"Toggle Child Menu" doesn't give enough context of the submenus, because they are related to the parent link. I suggest something like: "Hello World Child Menu". In this way, we know the menu we're opening is related to the parent link.
"Go to Top" Link
It should be inside the <footer> tag. More info: https://make.wordpress.org/themes/handbook/review/accessibility/required/#aria-landmark-roles.
Categories Link - Focus outline
We don't find an outline on a category link when focused. That can be seen on the homepage -> Post -> Published under: section.
Related Posts link - Focus
These links don't have the same visibility as the others we find on the website.
cc @rade1 @zex2911
Fix this and resubmit again.
Regards
KafleG