WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#25946 closed enhancement (fixed)

Twenty Fourteen: General code cleanup

Reported by: obenland Owned by:
Milestone: 3.8 Priority: normal
Severity: normal Version: 3.8
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

Major development is done, now follows the polishing phase.
Lets round up all the bits and pieces that can be shaved off to make Twenty Fourteen in all regards the exemplary theme that it is expected to be.

Attachments (19)

25946.diff (494 bytes) - added by obenland 3 years ago.
Remove unused class name.
25946.1.diff (2.7 KB) - added by obenland 3 years ago.
Simplify and prefix post thumbnail sizes.
25946.2.diff (5.0 KB) - added by iamtakashi 3 years ago.
25946.3.diff (747 bytes) - added by iamtakashi 3 years ago.
Remove a font weight 900 italic which is not necessary in the theme as it is.
25946.4.diff (7.2 KB) - added by obenland 3 years ago.
25946.5.diff (4.8 KB) - added by iamtakashi 3 years ago.
25946.6.diff (946 bytes) - added by iamtakashi 3 years ago.
pattern-dark.svg (1.4 KB) - added by iamtakashi 3 years ago.
25946.7.diff (1.3 KB) - added by morganestes 3 years ago.
Fixed theme tags to match the ones allowed (flexible-width, not responsive-width).
25946.8.diff (412 bytes) - added by morganestes 3 years ago.
Correct return type from twentyfourteen_font_url().
25946.9.diff (503 bytes) - added by morganestes 3 years ago.
Typo correction.
25946.10.patch (1.7 KB) - added by seanchayes 3 years ago.
25946.patch (1.1 KB) - added by defries 3 years ago.
25946.10.diff (564 bytes) - added by kovshenin 3 years ago.
25946.11.diff (778 bytes) - added by kovshenin 3 years ago.
25946.12.diff (1.1 KB) - added by kovshenin 3 years ago.
25946.13.diff (586 bytes) - added by kovshenin 3 years ago.
25946.14.diff (7.5 KB) - added by iamtakashi 3 years ago.
25946.15.diff (1.4 KB) - added by iamtakashi 3 years ago.

Download all attachments as: .zip

Change History (56)

@obenland
3 years ago

Remove unused class name.

@obenland
3 years ago

Simplify and prefix post thumbnail sizes.

#1 @lancewillett
3 years ago

In 26151:

Twenty Fourteen: remove unused class name, props obenland. See #25946.

#2 @lancewillett
3 years ago

In 26152:

Twenty Fourteen: simplify and prefix post thumbnail sizes, props obenland. See #25946.

#3 @lancewillett
3 years ago

In 26154:

Twenty Fourteen: fix tabs to spaces in two files. See #25946.

@iamtakashi
3 years ago

#4 @iamtakashi
3 years ago

Remove #main-content where it's not necessary.

@iamtakashi
3 years ago

Remove a font weight 900 italic which is not necessary in the theme as it is.

@obenland
3 years ago

#5 @lancewillett
3 years ago

In 26217:

Twenty Fourteen: remove main-content element where not necessary, props iamtakashi. See #25946.

#6 @lancewillett
3 years ago

In 26218:

Twenty Fourteen: remove a font weight of 900 italic which isn't needed. Props iamtakashi. See #25946.

#7 @lancewillett
3 years ago

In 26226:

Twenty Fourteen: fix an issue with keyboard navigation where you couldn't navigate to the previous image. Props obenland, see #25946.

#8 @lancewillett
3 years ago

In 26227:

Twenty Fourteen: general cleanup for spacing, inline comments. Props obenland, see #25946.

@iamtakashi
3 years ago

#9 @iamtakashi
3 years ago

Attached a patch.

  • Remove singular body class when a page is set to be the front page.
  • Minor style tweaks.

#10 @lancewillett
3 years ago

In 26249:

Twenty Fourteen: remove singular body class when a page is set to be the front page, and minor style tweaks. Props iamtakashi, see #25946.

@iamtakashi
3 years ago

#11 @iamtakashi
3 years ago

Minor style adjustment for navigations and a new lighter pattern-dark graphic which makes easier to change the background color of the featured image content.

#12 @lancewillett
3 years ago

In 26272:

Twenty Fourteen: minor style adjustments for navigation and a new lighter pattern-dark graphic which makes easier to change the background color of the featured image content. Props iamtakashi.

Also update stylesheet tags. See #25946.

@morganestes
3 years ago

Fixed theme tags to match the ones allowed (flexible-width, not responsive-width).

@morganestes
3 years ago

Correct return type from twentyfourteen_font_url().

@morganestes
3 years ago

Typo correction.

#13 @lancewillett
3 years ago

In 26325:

Twenty Fourteen: fix typos and a correct @return value, props morganestes. See #25946.

#14 @lancewillett
3 years ago

In 26326:

Twenty Fourteen: style cite element correctly when not in a blockquote. See #25946.

@seanchayes
3 years ago

#15 @seanchayes
3 years ago

I've added my efforts at consistent code / function calls for the themes js files and added sample header/comment/documentation at the top where it was missing.

Patch 25946.10 added

#16 @lancewillett
3 years ago

In 26332:

Twenty Fourteen: standardize comment blocks and jQuery function wrapper calls in JavaScript files. Props seanchayes for initial patch, see #25946.

#17 @TobiasBg
3 years ago

From what I can see, the standardized jQuery function wrapper in [26332] is possibly a change in behavior.

The new form is a self-invoking closure function with the advantage of being able to use $ in the function again.
The old form however is/was a short form of jQuery's ready() method (as in jQuery(document).ready(...)) which is used to be sure that the contained code is run once the DOM is ready.

Now, I'm not an expert in jQuery or JS, but I wanted to bring it up so that the JS experts can maybe weigh in. There's some jQuery code in those calls that queries some DOM elements, so I could imagine that waiting for the ready event is a good idea.

@defries
3 years ago

#18 @defries
3 years ago

Removing redundant __(.

#19 @lancewillett
3 years ago

In 26355:

Twenty Fourteen: remove redundant gettext call, props defries. See #25946.

#20 @chipbennett
3 years ago

  • Keywords has-patch added

@kovshenin
3 years ago

#21 @kovshenin
3 years ago

In 25946.10.diff: filtering false suggests the filter expects a boolean value, but we expect an array. Use an existing function instead.

@kovshenin
3 years ago

#22 follow-up: @kovshenin
3 years ago

In 25946.11.diff prefix the genericons stylesheet handle.

@kovshenin
3 years ago

#23 @kovshenin
3 years ago

In 25946.12.diff: both style.css and ie.css use Genericons, so let's not forget to declare the dependancy.

@kovshenin
3 years ago

#24 @kovshenin
3 years ago

In 25946.13.diff: wp_enqueue_scripts does not run in the admin, so twentyfourteen-lato is not registered and thus not enqueued in twentyfourteen_admin_fonts. Patch makes user Lato is enqueued in admin_print_scripts.

#25 follow-up: @kovshenin
3 years ago

Sorry for hi-jacking the thread like this. I'm looking at twentyfourteen_paging_nav and there's some pretty interesting stuff there: html_entity_decode, wp_parse_str, WP_Rewrite -- definitely something I wouldn't expect a default theme to do, and also page is the default base, but that can easily be changed, so instead of hard-coding it, we should use $wp_rewrite->pagination_base.

However, given all that magic going on there, I think we should remove it all and write our own simple pagination function for Twenty Fourteen. One that simply uses get_pagenum_link(). Another option would be to play nice and add support for WP-PageNavi, like many themes do. A third option would be to write a patch for a better paginate_links(). Thoughts?

#26 @iamtakashi
3 years ago

In 25946.14.diff: Update editor stylesheet to reflect recent changes made in the main stylesheet.

@iamtakashi
3 years ago

@iamtakashi
3 years ago

#27 @iamtakashi
3 years ago

In 25946.15.diff, Tweak the vertical space for list items in the primary and footer widgets.

#28 in reply to: ↑ 22 @obenland
3 years ago

Replying to kovshenin:

In 25946.11.diff prefix the genericons stylesheet handle.

IIRC, we decided to leave it un-prefixed to avoid duplication in case a third party uses Genericons (like Genericon'd or others).

#29 in reply to: ↑ 25 @obenland
3 years ago

Replying to kovshenin:

A third option would be to write a patch for a better paginate_links().

Yes, this. :) I was actually working on a patch for that during the WCLDN contributor day. But even if we'd move it into core, it would not be compatible with 3.6 and 3.7.

#30 @lancewillett
3 years ago

In 26560:

Twenty Fourteen: code cleanup, props kovshenin. See #25946.

  • Filtering twentyfourteen_has_featured_posts() false suggests the filter expects a boolean value, but we expect an array. Use an existing function instead.
  • In both style.css and ie.css use Genericons, so let's not forget to declare the dependancy.
  • Make sure Lato font is enqueued in admin_print_scripts.

#31 @lancewillett
3 years ago

In 26561:

Twenty Fourteen: update editor stylesheet to reflect recent changes made in the main stylesheet. Props iamtakashi, see #25946.

#32 @lancewillett
3 years ago

In 26562:

Twenty Fourteen: tweak the vertical space for list items in the primary and footer widgets. Props iamtakashi, see #25946.

#33 @lancewillett
3 years ago

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

Please open new tickets for new issues.

#34 @lancewillett
3 years ago

In [26672]:

Twenty Fourteen: remove trailing slashes on void elements such as meta and link, and remove type attribute from script element. See #24499.

#35 @lancewillett
3 years ago

In 26694:

Twenty Fourteen: remove invalid rel attributes from post thumbnail output, and fix a CSS typo. See #25946 and #25325.

#36 @lancewillett
3 years ago

In 26699:

Twenty Fourteen: remove unneeded code from image template, see #25946.

#37 @lancewillett
3 years ago

In 26810:

Twenty Fourteen: minor spacing and CSS formatting fixes. See #25946.

Note: See TracTickets for help on using tickets.