Make WordPress Themes

Opened 13 months ago

Last modified 6 weeks ago

#145023 reviewing theme

THEME: pally – 1.3

Reported by: pointsgmbh's profile pointsgmbh Owned by: kafleg's profile kafleg
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 kafleg
#145023 THEME: pally – 1.3 reviewing kafleg

(this ticket)


https://themes.svn.wordpress.org/pally/1.2/screenshot.png
Theme Check Results:

  • 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)

Bildschirmfoto 2023-08-04 um 15.41.05.png (185.8 KB) - added by Travel_girl 12 months ago.
Bildschirmfoto 2023-08-04 um 15.41.20.png (173.1 KB) - added by Travel_girl 12 months ago.

Download all attachments as: .zip

Change History (22)

#1 @kafleg
13 months ago

  • Owner set to kafleg
  • Status changed from new to reviewing

#2 @kafleg
13 months ago

  • Your theme doesn't support sub-menus? Only the main(parent) menu are displayed.
  • Deprecated: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in C:\laragon\www\wp612\wp-includes\kses.php on line 1729 (Check by enabling author info in the post)

Fix this and resubmit again.

Regards
KafleG

#3 @Travel_girl
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 @pointsgmbh
13 months ago

Hallo @Travel_girl, danke für dein Feedback. Ich schaue mir die Punkte an und melde mich wieder! Gruß, Kerstin

#5 @Travel_girl
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

#6 @pointsgmbh
13 months ago

Ah ok thanks! Sorry this is my first time submitting :)

#7 @Travel_girl
13 months ago

@pointsgmbh no problem! Thats way we try to educate

#8 @kemai
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 @Travel_girl
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 @kemai
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 @themetracbot
12 months ago

  • Summary changed from THEME: pally – 1.2 to THEME: pally – 1.3

pally - 1.3

An accessible &amp; 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 kafleg
#145023 THEME: pally – 1.3 reviewing kafleg

(this ticket)


https://themes.svn.wordpress.org/pally/1.3/screenshot.png
Theme Check Results:

  • 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 @pointsgmbh
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 @pointsgmbh
10 months ago

Hi @kafleg and @Travel_girl,
I just wanted to know if you are still reviewing the theme?

Best regards,
Kerstin

#14 @Travel_girl
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

#15 @pointsgmbh
10 months ago

Hey, no problem, I thought I'd just ask :)

Greets,
Kerstin

#16 @kemai
9 months ago

Hi @kafleg and @Travel_girl, are you still reviewing the theme?
Regards,
Kerstin

#17 @kafleg
8 months ago

@Travel_girl Would you please followup this ticket?

#18 @Travel_girl
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 @samuelsilvapt
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

This ticket was mentioned in Slack in #accessibility by samuelsilvapt. View the logs.


6 weeks ago

Note: See TracTickets for help on using tickets.