Make WordPress Core

Opened 13 years ago

Closed 10 years ago

#17209 closed defect (bug) (fixed)

Category list table breaks when custom post type label is too long

Reported by: andrewryno's profile andrewryno Owned by: helen's profile helen
Milestone: 4.0 Priority: normal
Severity: normal Version: 3.1
Component: Taxonomy Keywords: good-first-bug has-patch needs-testing
Focuses: administration Cc:

Description

Registered a post type which has a longer label than "posts" (called "resources"). List table listing the categories for that post type breaks slightly when a name is over a certain length. Picture attached.

Offending CSS is setting a 10% width to that column.

I can provide a patch. Though since the number of posts is centered, the heading is left aligned. If that column is widened, should the title be centered as well (since left-aligned may look weird at one point)?

Attachments (7)

DyvL.Screen shot 2011-04-21 at 13-37-42.png (30.3 KB) - added by andrewryno 13 years ago.
17209a.diff (735 bytes) - added by philipjohn 10 years ago.
17209b.diff (802 bytes) - added by philipjohn 10 years ago.
patch-17209-column-alignment-demonstration.jpg (100.6 KB) - added by seanchayes 10 years ago.
17209.diff (4.6 KB) - added by GaVrA 10 years ago.
17209.2.diff (5.3 KB) - added by GaVrA 10 years ago.
17209c.diff (1.3 KB) - added by seanchayes 10 years ago.

Download all attachments as: .zip

Change History (33)

#1 @scribu
13 years ago

I propose we don't use the post type label there at all, as it's not accurate anyway. Just use 'Count' or some other fixed string.

#2 @scribu
13 years ago

Related: #14076

#3 @andrewryno
13 years ago

Replacing the label with "Count" also wouldn't require any CSS changes. I can upload a patch later if we want to change it to that (or any other standardized word).

#5 @andrewryno
12 years ago

  • Keywords ui-feedback ux-feedback added

Fixed in [18875]? Rather than wrapping, it just gets hidden. Suggest closing, although there could possibly be a better option.

http://d.pr/zjpX

#6 @helenyhou
11 years ago

  • Component changed from UI to Administration
  • Keywords ui-feedback ux-feedback removed

[18875] was reverted, so not fixed there. However, I like the idea of "Count", given #14076/#14084.

#7 @jeremyfelt
10 years ago

  • Component changed from Administration to Taxonomy
  • Focuses administration added

#8 @wonderboymusic
10 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to 4.0

@philipjohn
10 years ago

#9 @philipjohn
10 years ago

  • Keywords needs-patch removed

My first patch! Yay :) At WordCamp Sheffield Contributor Day, by the way...

This patch adds the following;

  • Checks if the post type name is longer than 5 characters
  • If so, use '#' as the column header instead
  • Otherwise, uses the post type name

It still falls back to "Posts" if the post type object isn't available.

I tried using "Count" but more often than not the T disappeared producing a similar result to the example given for this bug, so didn't seem an appropriate solution.

Hash ('#') was chosen as something that is (probably) universally recognised as meaning "number"/"count". I'm not confident of that, hence why I've put it through the i18n function but thoughts welcome on that (hence why I haven't updated POT either).

#10 @johnbillion
10 years ago

  • Keywords has-patch added

@philipjohn
10 years ago

#11 @philipjohn
10 years ago

On s1m0nd's suggestion I've updated the patch to use the contextual gettext function.

#12 @seanchayes
10 years ago

Functionally the patch (17209b.diff) does what it sets out to do.

I created my custom post type and associated a taxonomy. While on the taxonomy edit screen (edit-tags.php) the number sign appeared when my custom post type name was longer than 5 characters. When I adjusted the length of the name it below 5 characters it displayed the name.

Talking to the point of andrewryno, cosmetically, the last column now looks a little out of horizontal alignment (see image) - the content alignment for the last column could perhaps be improved for a more consistent layout on that edit-tags.php screen. (perhaps a suggestion might be that .column-posts could be text-align:left;)

I did revert the patch and saw that when the slug name is used to title the column this alignment issue seemed to be less prevalent.

@GaVrA
10 years ago

@GaVrA
10 years ago

#13 @GaVrA
10 years ago

  • Keywords needs-testing added

First patch while working on it at WordCamp Switzerland during contributors day... :)

I attached 17209.2.diff which does the following:

  1. changes the label to # and makes it centered because it makes it consistent with other list table views, like Posts column on the wp-admin/users.php
  2. replaces generated html so there is only one span element inside the anchor and places triangle icon via :after pseudo-element to that span element
  3. applies some css changes so that the labels can be centered and the triangle icon is positioned absolutely next to the label.
  4. applies same css changes for RTL except different positioning, i.e. on the left side of the string
  5. it does not address accessibility concerns, so we might put title attribute with real post name, like Number of Posts
  6. removes some old css, I guess, for .sortable-indicator, both for LTR and RTL:
th.sorted.asc .sorting-indicator, 
th.desc:hover span.sorting-indicator { 
        display: block; 
        background-position: 0 0; 
} 
 
th.sorted.desc .sorting-indicator, 
th.asc:hover span.sorting-indicator { 
        display: block; 
        background-position: -7px 0; 
}

From my testing this works both on hovering over th.sortable and when orderby is applied via GET, i.e. when you click some th.sortable anchor. Also, I have tested RTL with plugin rtl-tester and everything is working.

Here are some screenshots using # before applying css changes:

small number
http://i.imgur.com/Uh8jq7G.png

large number
http://i.imgur.com/5bY8ifW.png

Some screenshots after applying css changes:

small number
http://i.imgur.com/QWc69zA.png

large number
http://i.imgur.com/KkM3U7f.png

when sorted
http://i.imgur.com/UxI7clf.png

RTL when sorted
http://i.imgur.com/8i7IFoS.png

#14 @helen
10 years ago

I still like the word "Count", which should fit just fine, and would be more accurate than continuing to put the post type name there, since that's not accurate. It shows the count across all post types. Doesn't matter where you're viewing it. That it's linked and can lead you to seeing that there are no posts in that category is another issue entirely.

Not sure center aligned is a good idea, but should definitely make the header and column align, I guess on the left for LTR.

GaVrA: for the future, RTL is generated in Grunt, so you don't need to patch it.

#15 @philipjohn
10 years ago

I'd tried "Count" (agree that would be better) but found that in many screen sizes both the C and T were partially or wholly cut off. I hadn't attempted to widen the column in the hopes of keeping it simple, but that could be revisited.

#16 @GaVrA
10 years ago

At around 800px screen size you can clearly see what philipjohn said:

http://i.imgur.com/bH3itIW.png

#17 @GaVrA
10 years ago

Oh and setting that count column to 80px pretty much fixes that issue on all screen sizes(on mobile it the triangle icon was breaking the layout).

#18 @seanchayes
10 years ago

I've refreshed 17209b.diff and within it I have set a fixed width on that last column and also reverted to titling it "Count".

I found that setting the width to 74px worked consistently across multiple devices sizes allowing the sorting triangle to appear in the same spot. 80px seemed a little wide.

17209c.diff contains these changes.

@seanchayes
10 years ago

#19 @nacin
10 years ago

  • Owner set to helen
  • Status changed from new to assigned

Assigning to Helen for review (and to remove this ticket from the good-first-bugs report).

#20 @SergeyBiryukov
10 years ago

I'm not a fan of conditionally changing the label here.

strlen() is not an accurate check for UTF-8 strings, and it looks like 'Count' is more correct (per comment:14), so I'd suggest removing the check and using 'Count' instead of the post type label.

#21 @GaVrA
10 years ago

What about smaller screens( comment:16 )?

#22 @SergeyBiryukov
10 years ago

The width issue appears to be resolved in the latest patch (per comment:18).

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


10 years ago

#24 @mordauk
10 years ago

Just tested 17209c.diff and it seems to work fine for me!

#26 @helen
10 years ago

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

In 29343:

Use the word "Count" instead of the post type label in taxonomy list tables to prevent layout issues. It is also more accurate, as it represents a count of all post types, not just one.

props philipjohn, GaVrA, seanchayes. fixes #17209.

Note: See TracTickets for help on using tickets.