Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 3 years ago

#6206 closed enhancement (fixed)

add class name to `th label` column in notification table

Reported by: danbp Owned by:
Milestone: Priority: normal
Severity: normal Version: 1.2
Component: Templates Keywords:
Cc:

Description

Seems there is a missing class name in the notification table TH.
/members/single/notifications/notifications-loop.php:6

<table class="notifications">
   <thead>
      <tr>
	<th class="icon">
	<th><label class="bp-screen-reader-text" 
        <th class="title">

Change History (6)

#1 @hnla
5 years ago

@danbp What missing class name? this table was updated on the last release but I don't see anything wrong with the template markup, can you clarify?

#2 @danbp
5 years ago

It's not wrong, but missing imho. The're 3 th in the excerpt.
1) th class"icon"
2) th <label class
3) th class "title"

A class name for this th can be usefull to define a width. Not sure if doing this for the label contained inside will be enough.
And to be logic, why have the other th a class name and this one has none ?

Normally, a cell takes up the space it needs to display the content. The width attribute is used to set a predefined width of a cell.

But what is "normal" with CSS ?

In english, this table cell contains "Select all". In other languages this string can be much longer, and maybe wraped into two lines, depending of the place where the table is used.

According to the language used, there is actually a more or less long string above a column who contains only checkboxes.

Personally i removed this string (but not the label to gain place and let fit the column to only the checkbox width. But this is my choice and not the reason of the ticket. ;-)

#3 @hnla
5 years ago

Ah point taken, yes the TH could have a class for consistency with it's siblings, and can be addressed.

The label string is actually hidden, or hidden by default in the default BP styles, perhaps you haven't those rules or have chosen to display?

As you say tables are designed to calculate their own widths as required by content, applying widths to override that behaviour may only, should only be done via css properties, generally '%' units or by using the colgroup/colspan elements. Using the sibling combinator might work in this instance though th.icon + th {} in light of lack of a class token.

#4 @DJPaul
5 years ago

  • Component changed from Component - Notifications to Appearance - Template Parts
  • Milestone changed from Awaiting Review to Future Release
  • Summary changed from missing class name in notification table to add class name to `th label` column in notification table
  • Type changed from defect (bug) to enhancement
  • Version changed from 2.2 to 1.2

#5 @DJPaul
3 years ago

  • Component changed from Appearance - Template Parts to Templates

#6 @slaFFik
3 years ago

  • Milestone Future Release deleted
  • Resolution set to fixed
  • Status changed from new to closed

Already done like this:

<th class="bulk-select-all">
    <input id="select-all-notifications" type="checkbox">
    <label class="bp-screen-reader-text"></label>
</th>

as part of #6289.

Note: See TracTickets for help on using tickets.