WordPress.org

Make WordPress Core

Opened 7 months ago

Closed 3 weeks ago

Last modified 3 weeks ago

#40138 closed enhancement (fixed)

Bundled themes: the tag cloud widget should output a list

Reported by: afercia Owned by: sami.keijonen
Milestone: 4.9 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: tag-cloud has-patch 2nd-opinion needs-testing
Focuses: ui, accessibility Cc:

Description

Splitting this out from #35566

Bundled themes should show best practices to theme authors. About the Tag Cloud widget, the latest patch on #35566 proposes some accessibility improvements, however it doesn't force the tags to be output as an unordered list to avoid the need to output some inline stylesheet.

A better option would be using the format argument to tweak the output on a case by case basis.

From a semantic perspective, the tags list is... well, a list. Setting the format argument to list would allow assistive technologies to announce the list as a list and the number of items in the list. Most of the users from the a11y testers group expressed a preference to have the number of items being announced, see the test results on #35566.

This would require some simple CSS adjustments, which is trivial to do in the Themes stylesheet.

Attachments (14)

40138.2015.diff (1.4 KB) - added by xkon 3 months ago.
TwentyFifteen ul fix for tag cloud
40138.2016.diff (1.2 KB) - added by xkon 3 months ago.
TwentyFifteen ul fix for tag cloud
40138.2014.diff (1.5 KB) - added by xkon 3 months ago.
Twenty Fourteen ul fix for tag cloud
40138.2013.diff (1.6 KB) - added by xkon 3 months ago.
Twenty Thirteen ul fix for tag cloud
40138.2012.diff (1.6 KB) - added by xkon 3 months ago.
Twenty Twelve ul fix for tag cloud
40138.2011.diff (1.4 KB) - added by xkon 3 months ago.
Twenty Eleven ul fix for tag cloud
40138.2011_1.diff (1.4 KB) - added by xkon 3 months ago.
Twenty Eleven ul fix for tag cloud - Updated function name.
40138.2010.diff (1.4 KB) - added by xkon 3 months ago.
Twenty Ten ul fix for tag cloud
40138_2010-2016.diff (10.2 KB) - added by xkon 3 months ago.
Bundle Patch for 2010 up to 2016 themes
40138.diff (10.2 KB) - added by xkon 3 months ago.
Standards Update.
40138.2.diff (10.3 KB) - added by xkon 4 weeks ago.
updated versions and css fixes
Screen Shot 2017-10-03 at 7.08.29 PM.png (62.2 KB) - added by davidakennedy 3 weeks ago.
Twenty Eleven Before
Screen Shot 2017-10-03 at 7.08.29 PM.2.png (62.2 KB) - added by davidakennedy 3 weeks ago.
Twenty Eleven After
40138.3.patch (14.2 KB) - added by davidakennedy 3 weeks ago.
Adds changes from https://core.trac.wordpress.org/ticket/40138#comment:26

Download all attachments as: .zip

Change History (44)

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


7 months ago

#2 @afercia
7 months ago

See also ticket:35566#comment:32, it would be nice to have Twenty Seventeen filtering the Tag Cloud arguments instead of overriding the styles.

#3 @sami.keijonen
7 months ago

I'd say lets filter Tag Cloud arguments in the same way as in Twenty16. With small CSS tweaks we can make it look the same as now.

However this could break child themes styling for tag cloud. But I think it's more important to have more accessible tag cloud. Child themes can fix minor CSS.

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


7 months ago

#5 @afercia
7 months ago

  • Keywords tag-cloud added
  • Milestone changed from Awaiting Review to 4.8

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


6 months ago

#7 @jbpaul17
5 months ago

  • Keywords needs-patch added

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


5 months ago

#9 @afercia
5 months ago

  • Milestone changed from 4.8 to 4.8.1

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


5 months ago

#11 @afercia
4 months ago

  • Milestone changed from 4.8.1 to 4.9

This ticket was mentioned in Slack in #core by xkon. View the logs.


3 months ago

@xkon
3 months ago

TwentyFifteen ul fix for tag cloud

#13 @xkon
3 months ago

Hello, I'll keep my diffs split up per theme to have easier access on them.

Also before everything to have a list of what is fixed

TwentySeventeen : @sami.keijonen in https://core.trac.wordpress.org/ticket/40184

Following the previous fix for TwentySeventeen to have similar code everywhere :

40138.2010.diff addresses the widget tagcloud fix for Twenty Ten.

40138.2011_1.diff addresses the widget tagcloud fix for Twenty Eleven.

40138.2012.diff addresses the widget tagcloud fix for Twenty Twelve.

40138.2013.diff addresses the widget tagcloud fix for Twenty Thirteen.

40138.2014.diff addresses the widget tagcloud fix for Twenty Fourteen.

40138.2015.diff addresses the widget tagcloud fix for Twenty Fifteen.

40138.2016.diff addresses the widget tagcloud fix for Twenty Sixteen ( sorry for the wrong name in the uploaded file, no idea how to edit that ).

I'll move on to the other themes uploading each patch with .YEAR respectively for each theme asap.

Best regards,
Konstantinos

Last edited 3 months ago by xkon (previous) (diff)

@xkon
3 months ago

TwentyFifteen ul fix for tag cloud

@xkon
3 months ago

Twenty Fourteen ul fix for tag cloud

@xkon
3 months ago

Twenty Thirteen ul fix for tag cloud

@xkon
3 months ago

Twenty Twelve ul fix for tag cloud

@xkon
3 months ago

Twenty Eleven ul fix for tag cloud

@xkon
3 months ago

Twenty Eleven ul fix for tag cloud - Updated function name.

@xkon
3 months ago

Twenty Ten ul fix for tag cloud

@xkon
3 months ago

Bundle Patch for 2010 up to 2016 themes

#14 @xkon
3 months ago

  • Keywords has-patch 2nd-opinion needs-testing added; needs-patch removed

Hello again,

I've uploaded a bundled patch with all the diffs for themes 2010 up to 2016 file: 40138_2010-2016.diff .

The other diffs are meant for single testing ( note: they might have differences from the final bundle, unfortunately I'm working remotely so it was difficult a bit to keep up everything in line with my pcs).

Hopefully this is close enough to address the matter at hand and proceed with the Admin Tag Cloud fixes as well.

Best regards,
Konstantinos

This ticket was mentioned in Slack in #core by xkon. View the logs.


3 months ago

@xkon
3 months ago

Standards Update.

#16 @xkon
3 months ago

I uploaded the final 40138.diff patch as I'm going through all of them to update to coding standards. My editor was a bit problematic 'on save' and I didn't realize till it was too late.

Sorry for the inconvenience if any was caused.

Best regards,
Konstantinos

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


3 months ago

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


3 months ago

#19 @afercia
4 weeks ago

  • Owner set to sami.keijonen
  • Status changed from new to assigned

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


4 weeks ago

#21 @sami.keijonen
4 weeks ago

I tested with 40138.diff and here are my notes. I'll note the version number also but I'm not sure how you handle it.

Twenty Sixteen

  • There is left margin on the UL-list which changes the design a little bit.
  • There is also bottom margin but that doesn't change design.
  • Update version to @since Twenty Sixteen 1.4.

Twenty Fifteen

  • Looks good to me
  • Update version to @since Twenty Fifteen 1.9.

Twenty Fourteen

  • Looks good to me
  • Update version to @since Twenty Fourteen 2.1.

Twenty Thirteen

  • .widget li gives 5px padding top and bottom. I'd probably give some spacing but on the top and right as in previous. Something like:
    .tagcloud ul li {
    padding: 4px 4px 0 0;
    }
    
  • Update version to @since Twenty Thirteen 2.3.

Twenty Twelve

  • .widget-area .widget li will give extra line-height. Font-size doesn't have an effect.
  • Update version to @since Twenty Twelve 2.4.

Twenty Eleven

  • Looks good to me.
  • Update version to @since Twenty Eleven 2.7.

Twenty Ten

  • Looks good to me.
  • Update version to @since Twenty Ten 2.4.

@xkon Do you happen to have time to fix these? Wait for a while anyways before David have reviewed this also.

@xkon
4 weeks ago

updated versions and css fixes

#22 @xkon
4 weeks ago

Thanks for the reply @sami.keijonen ! I've updated the patch to 40138.2.diff to have it ready for David as well as this is more accurate.

One note I've actually added a padding: 0; to TwentyThirteen this way it stays 100% the same as the original to be safe for now. Part from that everything else is updated according to your information.

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


3 weeks ago

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


3 weeks ago

@davidakennedy
3 weeks ago

Twenty Eleven Before

#25 @davidakennedy
3 weeks ago

@xkon @sami.keijonen Thanks for all the awesome work here! I started testing this, and noticed we'll need styles for RTL since some of the additions add margins and such that would need to be accounted for in the RTL styles.

#26 @davidakennedy
3 weeks ago

Hi @xkon and @sami.keijonen:

Thanks again for the work here. With some small tweaks, this should be ready. After some further testing, here's my feedback.

One thing to note is each of most of these themes has a "Widgets" section of the stylesheet. That would be a good place to put these styles to keep things consistent. Also, it's good to test the widget in multiple places.

Ten

  • Put new styles into widgets section of stylesheet.
  • Add RTL styles. Like:
#main .widget-area .tagcloud ul,
.widget-area .tagcloud ul {
	margin-right: 0;
}

Eleven

  • Put new styles into widgets section of stylesheet.

Twelve

  • Cancel out top margin on li items on front page template footer widget area. Like:
.template-front-page .widget-area .widget.widget_tag_cloud li {
	margin: 0;
}
  • Put new styles into widgets section of stylesheet.

Thirteen

  • Put new styles into widgets section of stylesheet.

Fourteen

  • Put new styles into widgets section of stylesheet, in list style widgets
  • I don't think the margins on the li items are needed. It makes the tag cloud appear too different than the previous design. Without that, no RTL styles are needed.

Fifteen

  • Put new styles into widgets section of stylesheet, in list style widgets
  • I don't think the margins on the li items are needed. It makes the tag cloud appear too different than the previous design. Without that, no RTL styles are needed.

Sixteen

Looks good! A few small tweaks:

*The comment "/* Tag Cloud UL Style Fixes */" isn't needed because it's just additional styles.

  • Add RTL styles. Like:
.tagcloud ul {
	margin-right: 0;
}

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 weeks ago

#28 @davidakennedy
3 weeks ago

In 40138.3, I added the feedback from my last comment, bouncing off the great work from @xkon.

#29 @SergeyBiryukov
3 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 41756:

Bundled Themes: Change tag cloud format to a list (<ul>) for better semantics and accessibility.

List markup allows screen reader users to know in advance how many tags are within the list.

Props xkon, davidakennedy.
Fixes #40138.

#30 @SergeyBiryukov
3 weeks ago

In 41757:

Bundled Themes: Restore double line breaks before media queries.

See #40138.

Note: See TracTickets for help on using tickets.