Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#49549 closed defect (bug) (fixed)

Bundled Themes: Update calendar widget styles for 5.4 markup

Reported by: ianbelanger's profile ianbelanger Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch commit dev-reviewed
Focuses: css Cc:

Description

As pointed out in #49546 by @sabernhardt for Twenty Thirteen.

Since the previous month and next month links are no longer inside table cells in version 5.4, display:block stretches each of those links the full width of the calendar container.

Though I like how the month links are larger and further away from the calendar date links, the change would be quite unexpected for other people.

(markup was changed for ticket:39763)

It appears that all other Bundled Themes are also affected by the markup changes to the calendar widget in 5.4.

Attachments (4)

49549.patch (82.1 KB) - added by sabernhardt 5 years ago.
editing remaining 9 bundled themes, without borders and background colors
49549.1.patch (84.2 KB) - added by sabernhardt 5 years ago.
with borders and background colors
2013-single-month-link-focus.png (6.7 KB) - added by sabernhardt 5 years ago.
Calendar with only the next-month link, in Twenty Thirteen side widget area, with focus outline to show its size
49549.2.patch (3.8 KB) - added by sabernhardt 5 years ago.
adjusting span widths

Download all attachments as: .zip

Change History (16)

#1 @sabernhardt
5 years ago

Here are screenshots for each of the bundled themes:
https://flic.kr/s/aHsmLF43dM

The screenshots include Twenty Thirteen, which already has a patch committed. (Thanks!)

@sabernhardt
5 years ago

editing remaining 9 bundled themes, without borders and background colors

@sabernhardt
5 years ago

with borders and background colors

#2 @sabernhardt
5 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

Borders and background colors are left out of the first patch (child themes may have overridden color choices and then would need to be revised). The second patch includes them, in case that's preferable.

In both patches, all themes:

  • Use display:table on the nav element and display:table-cell on the span elements inside it
  • Set the width to 100% (except for Twenty Eleven at 95%)

Theme-specific changes include:

Twenty Twenty

  • Adjusts font-size to 1.8rem on navigation element
  • Uses same styles for calendar's table cells on the navigation span elements (font-size, font-weight, line-height, padding, text-align)
  • Removes bottom margin from calendar table
  • Removes underline from the links, as specified before
  • Aligns month links to the outside edges
  • Second patch adds side and bottom borders (setting border-color in functions.php)

Twenty Nineteen
Note: these stylesheets were edited separately from the SCSS file, not compiled, if that makes a difference.

  • Moves the bottom margin from the calendar table to the nav element
  • Adds the heading font to navigation links, and uses the non-Latin font fallbacks for 31 languages
  • Adds 0.5em padding to the span tags so they match the cells

Twenty Seventeen

  • Moves the bottom margin from the calendar table to the nav element
  • Centers the navigation links
  • Sets a width (15%) to the center ".pad" span
  • Adds 0.4em top and bottom padding to the nav element
  • Second patch adds bottom border

Twenty Sixteen

  • Matches calendar table cells' line-height, centering and lack of padding, then adjusts line-height with the cells at (min-width: 56.875em)
  • Sets a width (15%) to the center ".pad" span
  • Second patch adds side and bottom borders

Twenty Fifteen

  • Sets a width (15%) to the center ".pad" span
  • Matches calendar table cells' line-height, centering and lack of padding, then adjusts line-height with the cells at the five larger breakpoints
  • Moves the bottom margin from the calendar table to the nav element at (min-width: 46.25em)
  • Second patch adds side and bottom borders

Twenty Fourteen

  • Adjusts navigation line-height to 2
  • Aligns text for previous and next months to outside edges
  • Second patch adds side and bottom borders, with box-sizing adjustment

Twenty Twelve

  • Gives the same styles to .wp-calendar-nav as were assigned to the #wp-calendar table (margin, width, font-size, line-height, color)
  • Adds side padding to the next link container span (.wp-calendar-nav-next) to match the former tfoot cell

Twenty Eleven

  • Centers the navigation links
  • Second patch adds background color plus top and bottom borders

Twenty Ten

  • Assigns fonts to .calendar_wrap container, to match #wp-calendar
  • Aligns text for next month links to the outside edges

.

Last edited 5 years ago by sabernhardt (previous) (diff)

#3 @ianbelanger
5 years ago

  • Status changed from assigned to reviewing

Thanks for your patches @sabernhardt! I am going to review both for commit. I'll most likely commit the first one, but thanks for the options.

#4 @ianbelanger
5 years ago

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

In 47405:

Bundled Themes: Update calendar widget styles for 5.4 markup.

Fixes the alignment of month links in the calendar widget with the changes in version 5.4 for all Bundled Themes, except Twenty Thirteen, which was already fixed in #49546.

Props sabernhardt.
Fixes #49549.

#5 @ianbelanger
5 years ago

  • Keywords fixed-major added
  • Milestone changed from 5.4 to 5.3.3
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport.

#6 @ocean90
5 years ago

@ianbelanger Why does this need a backport if it's related to 5.4?

#7 @ianbelanger
5 years ago

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

Sorry, you are correct. I'm still new to committing.

#8 @sabernhardt
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@ianbelanger I'm reopening this ticket because I had failed to test how the table/table-cell layout works with only one month link. In that case, which again is most noticeable with Twenty Thirteen, the link takes up about 80% of the table width (about 6 columns).

It seems the spans should have a specific (percentage) width for the themes where that element has centered text. 49549.2.patch sets the previous and next spans to 40% in those six themes. Those container elements had been three-sevenths (42.86%) as table footer cells, and I rounded down for a better chance of even pixel values.

Simply setting previous and next links' width at 40%:

  1. Twenty Eleven
  2. Twenty Thirteen
  3. Twenty Nineteen

Setting previous and next width at 40% instead of the 15% .pad width I used earlier:

  1. Twenty Fifteen
  2. Twenty Sixteen
  3. Twenty Seventeen

I've tested 49549.2.patch:

  • with each bundled theme,
  • in 4 PC browsers (Chrome, Firefox, IE11, and Edge),
  • in English and Hebrew (LTR and RTL languages), and
  • with each link set (previous month only, next month only, and both).

Side note: Twenty Twenty can lose some table border lines in Internet Explorer and Edge, but it already did that in 5.3 (so I'm not addressing that here).

If the width issue is minor enough, this edit could be punted to 5.5 (or possibly even ignored).

@sabernhardt
5 years ago

Calendar with only the next-month link, in Twenty Thirteen side widget area, with focus outline to show its size

@sabernhardt
5 years ago

adjusting span widths

#9 @ianbelanger
5 years ago

  • Keywords needs-dev-review added; needs-testing removed
  • Owner changed from ianbelanger to SergeyBiryukov
  • Status changed from reopened to reviewing

Patch looks good to me @sabernhardt, I will commit, just need a dev-review from another committer because we are in the RC stage. @SergeyBiryukov would mind reviewing this one? Thanks

#10 @SergeyBiryukov
5 years ago

  • Keywords commit dev-reviewed added; needs-dev-review removed

49549.2.patch looks good to me.

#11 @SergeyBiryukov
5 years ago

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

In 47444:

Bundled Themes: Update calendar widget styles for 5.4 markup to fix the alignment of a single month link.

Follow-up to [47405].

Props sabernhardt.
Reviewed by ianbelanger, SergeyBiryukov.
Fixes #49549.

#12 @SergeyBiryukov
5 years ago

In 47445:

Bundled Themes: Update calendar widget styles for 5.4 markup to fix the alignment of a single month link.

Follow-up to [47405].

Props sabernhardt.
Reviewed by ianbelanger, SergeyBiryukov.
Merges [47444] to the 5.4 branch.
Fixes #49549.

Note: See TracTickets for help on using tickets.