Make WordPress Core

Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#40138 closed enhancement (fixed)

Bundled themes: the tag cloud widget should output a list

Reported by: afercia's profile afercia Owned by: samikeijonen's profile 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 7 years ago.
TwentyFifteen ul fix for tag cloud
40138.2016.diff (1.2 KB) - added by xkon 7 years ago.
TwentyFifteen ul fix for tag cloud
40138.2014.diff (1.5 KB) - added by xkon 7 years ago.
Twenty Fourteen ul fix for tag cloud
40138.2013.diff (1.6 KB) - added by xkon 7 years ago.
Twenty Thirteen ul fix for tag cloud
40138.2012.diff (1.6 KB) - added by xkon 7 years ago.
Twenty Twelve ul fix for tag cloud
40138.2011.diff (1.4 KB) - added by xkon 7 years ago.
Twenty Eleven ul fix for tag cloud
40138.2011_1.diff (1.4 KB) - added by xkon 7 years ago.
Twenty Eleven ul fix for tag cloud - Updated function name.
40138.2010.diff (1.4 KB) - added by xkon 7 years ago.
Twenty Ten ul fix for tag cloud
40138_2010-2016.diff (10.2 KB) - added by xkon 7 years ago.
Bundle Patch for 2010 up to 2016 themes
40138.diff (10.2 KB) - added by xkon 7 years ago.
Standards Update.
40138.2.diff (10.3 KB) - added by xkon 7 years ago.
updated versions and css fixes
Screen Shot 2017-10-03 at 7.08.29 PM.png (62.2 KB) - added by davidakennedy 7 years ago.
Twenty Eleven Before
Screen Shot 2017-10-03 at 7.08.29 PM.2.png (62.2 KB) - added by davidakennedy 7 years ago.
Twenty Eleven After
40138.3.patch (14.2 KB) - added by davidakennedy 7 years 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.


8 years ago

#2 @afercia
8 years 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
8 years 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.


8 years ago

#5 @afercia
8 years 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.


8 years ago

#7 @jbpaul17
8 years ago

  • Keywords needs-patch added

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


8 years ago

#9 @afercia
8 years ago

  • Milestone changed from 4.8 to 4.8.1

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


7 years ago

#11 @afercia
7 years ago

  • Milestone changed from 4.8.1 to 4.9

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


7 years ago

@xkon
7 years ago

TwentyFifteen ul fix for tag cloud

#13 @xkon
7 years 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 7 years ago by xkon (previous) (diff)

@xkon
7 years ago

TwentyFifteen ul fix for tag cloud

@xkon
7 years ago

Twenty Fourteen ul fix for tag cloud

@xkon
7 years ago

Twenty Thirteen ul fix for tag cloud

@xkon
7 years ago

Twenty Twelve ul fix for tag cloud

@xkon
7 years ago

Twenty Eleven ul fix for tag cloud

@xkon
7 years ago

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

@xkon
7 years ago

Twenty Ten ul fix for tag cloud

@xkon
7 years ago

Bundle Patch for 2010 up to 2016 themes

#14 @xkon
7 years 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.


7 years ago

@xkon
7 years ago

Standards Update.

#16 @xkon
7 years 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.


7 years ago

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


7 years ago

#19 @afercia
7 years 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.


7 years ago

#21 @sami.keijonen
7 years 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
7 years ago

updated versions and css fixes

#22 @xkon
7 years 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.


7 years ago

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


7 years ago

@davidakennedy
7 years ago

Twenty Eleven Before

#25 @davidakennedy
7 years 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
7 years 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.


7 years ago

#28 @davidakennedy
7 years ago

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

#29 @SergeyBiryukov
7 years 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
7 years ago

In 41757:

Bundled Themes: Restore double line breaks before media queries.

See #40138.

Note: See TracTickets for help on using tickets.