W3C Error with get_calendar()

[Harry Milatz]

Hi @all,

The W3C Validator reports an error:
Element tbody not allowed as child of element table in this context.

If the tbody tag will be added to

$calendar_output

before the tfoot tag is added, the error is gone.

Best regards,
Harry

[Harry Milatz]

"correctet" file without produce the error

[desrosj]

Hi @harry-milatz,

My apologies that this ticket fell through the cracks! Are you able to ​create a proper patch with the suggested changes?

I just did some testing using the latest version of WordPress and Twenty Nineteen and it seems this issue still exists. After some searching, it seems that the <tfoot> element was allowed to precede the <tbody> element ​in HTML 5. However, that was ​changed in HTML 5.1 and <tfoot> must now follow <tbody>.

I am tagging this for accessibility review because moving the navigation links in <tfoot> below the table's body will likely have implications for users utilizing assistive technologies (a user may need to navigate through the entire calendar month even if it is not the desired month).

The implications this change has for CSS in themes must also be considered and tested to ensure no strangeness comes up.

[abhijitrakas]

@desrosj

I have made changes accordingly. Please check "39763.diff" file for changes. Please let me know if it needs any more changes.

Thanks,

[SergeyBiryukov]

Tested 39763.diff​, looks good to me.

Tentatively marking for commit, pending feedback from the Accessibility team.

[SergeyBiryukov]

Per ​feedback from @joedolson in #accessibility:

the tfoot element should not contain controls - it should only contain <th> cells that act as headings. So, yes, I'd call that a problem.
The previous/next links really shouldn't be inside the table.

Looks like some investigation is needed on moving those links out of the table and how many themes having styles for the widget would be affected by that.

[isabel_brison]

Hey @SergeyBiryukov , I notice this ticket is assigned to you but also has needs-patch keyword; is it OK to pick up or are you working on it? Thanks!

[SergeyBiryukov]

Replying to isabel_brison:

I notice this ticket is assigned to you but also has needs-patch keyword; is it OK to pick up or are you working on it?

Feel free to pick it up, not actively working on it now :)

[isabel_brison]

Proposed change after a11y feedback from Nov 2019

[isabel_brison]

I have added a patch that removes the <tfoot> altogether, and moves the "previous" and "next" month links into a <nav> that sits below the table. I couldn't think of a good way to semantically link this nav to the table, but they both sit under the same widget heading. Would be good to have some more accessibility feedback on this.

In terms of theme styling, this solution may look slightly broken with existing themes if they were depending on <tfoot> borders (e.g. Twenty Fourteen) or if they add generic styles to the <table> element (e.g Twenty Nineteen with the font family). Otherwise, for layout themes generally depend on the table layout itself, and that will be easy enough to replicate with a bit of CSS.

[MarcoZ]

I like that the table loses some of its layout table characteristic here and that the navigational links are in a proper nav element. One thing I noted is that the aria-label for the nav element needs to be localizable. Right now the string is hardcoded.

One question: While we're here, are there things in the thead that might also be more fitting outside the table?

[audrasjb]

Adds i18n string and fix some WPCS issues

[audrasjb]

Before patch

[audrasjb]

After patch

[audrasjb]

Hi @isabel_brison thanks for the patch!

I refreshed the patch to add correct internationalization and to fix some minor WPCS issues with white spaces.

I tested the patch with bundled themes and it looks good to me.
Adding needs-dev-note keyword since theme authors may want to know about this change.

[isabel_brison]

@MarcoZ thanks for the feedback! The thead only contains the weekday as a heading for each of the columns, which I believe is semantically correct. There are no links in it. Does anything seem off to you?

@audrasjb thanks for fixing up the patch! Is it good to go now, or is there anything else I can do to progress this ticket?

[audrasjb]

Marking it for commit to get final review and commit action :-)

[jorbin]

In 47223:

General: Fix HTML for get_calendar()

The <tfoot> element was allowed to precede the <tbody> element in HTML 5. However, that was changed in HTML 5.1 and <tfoot> must now follow <tbody>.

This removes the tfooot and instead uses a nav element for navigation between months.

Fixes: #39763
Props: isabel_brison, Harry Milatz, audrasjb, abhijitrakas, SergeyBiryukov

[jorbin]

Reopening because this needs a devnote.

[audrasjb]

Closing this ticket as fixed: it has needs-dev-note keyword and it's on our list for 5.4’s dev notes :)

[BackuPs]

Hi Guys

Can you also add a unique class name to the nav wrapper, nav prev and nav next to make it easier to target them by css class name instead of by id and html element?

Thank you

[sabernhardt]

adds classes to new elements, and removes "pad" class from empty #next span

[sabernhardt]

This patch adds classes to the new nav and span elements:

<nav class="wp-calendar-months" ...>
<span id="prev" class="wp-calendar-previous-month">
<span id="next" class="wp-calendar-next-month">

I checked ​https://wpdirectory.net/ to make sure none of these names were already in one of those plugins or themes. If there's a better choice, though, feel free to change each name.

Note also: I expect that removing the "pad" class from the empty previous month's span tag was intentional. So I removed it from the empty next month's span as well. (Please correct if I'm wrong.)

[audrasjb]

Thanks for the suggestion @BackuPs and thank your for the patch @sabernhardt :-)

In 39763.5.diff I'd suggest to use the following classes, for better clarity:

• .wp-calendar-nav for the navigation wrapper
• .wp-calendar-nav-prev for the previous month link
• .wp-calendar-nav-next for the next month link

Cheers,
Jb

[audrasjb]

Widgets: Add CSS classes to calendar widget navigation

[audrasjb]

Testing 39763.5.diff

[audrasjb]

Patch tested, it works fine for me.
@jorbin as the initial committer of the ticket, that would be wonderful if you were available to proof-review and hopefully commit this small patch to add classes to the navigation elements. Thanks!

[sabernhardt]

@audrasjb Thanks! I prefer your class names over mine.

[jorbin]

While we do that, I wonder if we should also remove the ids as it is possible multiple calendars could be used on one page. What do others think?

[audrasjb]

Looks like a great idea!

[afercia]

This ticket was discussed during today's accessibility bug-scrub: it would be great to have the patch updated as per last request to give this ticket a change to be included in 5.4.

[sabernhardt]

removes #prev and #next IDs

[sabernhardt]

I briefly hesitated to remove the prev and next IDs because they are used by themes as selectors. However, if theme authors want to match the former styling, they probably need to update their stylesheets for the new elements anyway.

So it probably would be better to remove those IDs now.

[sabernhardt]

I'd like to propose adding one more class, to the table:
<table id="wp-calendar" class="wp-calendar-table">

Then that ID might be removed later (not yet, though, because thousands of themes use that to style the table).

[sabernhardt]

adds class to wp-calendar table

[SergeyBiryukov]

In 47374:

General: Introduce HTML classes in get_calendar() for easier CSS targeting:

• .wp-calendar-table for the <table> element.
• .wp-calendar-nav for the navigation wrapper.
• .wp-calendar-nav-prev for the previous month link.
• .wp-calendar-nav-next for the next month link.

Replace #prev and #next HTML IDs with .wp-calendar-nav-prev and .wp-calendar-nav-next classes.

Props sabernhardt, BackuPs, audrasjb, jorbin.
Fixes #39763.

[audrasjb]

For information, the existing dev note was updated to reflect those enhancements.