Make WordPress Core

Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#44004 closed defect (bug) (fixed)

Twenty Seventeen: CSS issue when using rowspan in tables

Reported by: aussi's profile aussi Owned by: ianbelanger's profile ianbelanger
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-screenshots has-patch
Focuses: Cc:

Description

I've spotted an issue with the TwentySeventeen theme. This definition in styles.css:

th:first-child,
td:first-child {
    padding-left: 0;
}

causes problems when using tables containing rowspans. Specifically, this code:

<table width="100%">
  <thead>
    <tr>
      <th>Head 1</th>
      <th>Head 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <td rowspan="3">Cell 1</td>
      <td>Cell 2</td>
    </tr>
    <tr>
      <td>Cell 3</td>
    </tr>
    <tr>
      <td>Cell 4</td>
    </tr>
  </tbody>
</table>

produces a table where Head 2 and Cell 2 have padding-left: 0.4em, while Cell 3 and Cell 4 have padding-left: 0. This produces a table with misaligned cells in the same column.

Attachments (2)

screenshot.png (2.8 KB) - added by subrataemfluence 6 years ago.
44004.diff (347 bytes) - added by jainnidhi 6 years ago.

Download all attachments as: .zip

Change History (17)

#1 @SergeyBiryukov
6 years ago

  • Component changed from Themes to Bundled Theme

#2 @subrataemfluence
6 years ago

  • Keywords has-screenshots added

We have a 0.4em padding set for all td elements, which makes the content to stay nicely inside the td with an equal small padding all sides.

td {
   padding: 0.4em;
}

But since the pseudo elements :first-child and :last-child have a left padding set to 0 just below the above rule, I feel we need to apply the same for td and th as well. This will ensure all elements would have no left padding but have a padding of 0.4em on other three sides, i.e. top, right and bottom.

The following works for me.

th:first-child,
td:first-child,
td,
th {
   padding-left: 0;
}

@jainnidhi
6 years ago

#3 @jainnidhi
6 years ago

  • Keywords has-patch added

#4 @SergeyBiryukov
6 years ago

  • Summary changed from TwentySeventeen Theme - CSS issue when using rowspan in tables to Twenty Seventeen: CSS issue when using rowspan in tables

#5 @laurelfulford
5 years ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to Future Release

#6 @ianbelanger
5 years ago

  • Keywords needs-testing removed

Just tested the patch and it does fix the issue on the front-end. It does not work in the editor though. However, this may not matter because the Table Block does not allow you to use rowspan, it gets stripped out or you have to convert it to an HTML Block.

#7 @ianbelanger
5 years ago

  • Keywords 2nd-opinion added

#8 @ianbelanger
5 years ago

  • Milestone changed from Future Release to 5.3
  • Severity changed from minor to normal
  • Version 4.9.5 deleted

#9 @davidbaumwald
5 years ago

  • Milestone changed from 5.3 to Future Release

Based on the latest comments, this ticket still needs some work. With Beta 1 of version 5.3 only a few days away, this is being moved to Future Release. If this ticket can be resolved in time for 5.3, feel free to re-milestone.

#10 @ianbelanger
4 years ago

  • Keywords needs-testing added; 2nd-opinion removed
  • Milestone changed from Future Release to 5.5

#11 @ianbelanger
4 years ago

  • Keywords needs-testing removed
  • Owner set to ianbelanger
  • Status changed from new to reviewing

Reviewing for commit.

#12 @ianbelanger
4 years ago

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

In 48028:

Bundled Themes: Twenty Seventeen CSS issue when using rowspan in tables.

Fixes CSS issue when using rowspan in tables. This issue does not apply to the Table Block, just tables inserted as custom html.

Props aussi, subrataemfluence, jainnidhi.
Fixes #44004.

#13 @ianbelanger
4 years ago

  • Keywords fixed-major added
  • Milestone changed from 5.5 to 5.4.3
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport.

#14 @desrosj
4 years ago

  • Keywords fixed-major removed
  • Resolution set to fixed
  • Status changed from reopened to closed

With 5.5 just under 5 weeks away, there is no 5.4.3 planned. Moving back to the 5.5 milestone.

#15 @desrosj
4 years ago

  • Milestone changed from 5.4.3 to 5.5
Note: See TracTickets for help on using tickets.