WordPress.org

Make WordPress Core

Opened 22 months ago

Closed 21 months ago

Last modified 21 months 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 22 months ago.
Remove unused class name.
25946.1.diff (2.7 KB) - added by obenland 22 months ago.
Simplify and prefix post thumbnail sizes.
25946.2.diff (5.0 KB) - added by iamtakashi 22 months ago.
25946.3.diff (747 bytes) - added by iamtakashi 22 months 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 22 months ago.
25946.5.diff (4.8 KB) - added by iamtakashi 22 months ago.
25946.6.diff (946 bytes) - added by iamtakashi 22 months ago.
pattern-dark.svg (1.4 KB) - added by iamtakashi 22 months ago.
25946.7.diff (1.3 KB) - added by morganestes 22 months ago.
Fixed theme tags to match the ones allowed (flexible-width, not responsive-width).
25946.8.diff (412 bytes) - added by morganestes 22 months ago.
Correct return type from twentyfourteen_font_url().
25946.9.diff (503 bytes) - added by morganestes 22 months ago.
Typo correction.
25946.10.patch (1.7 KB) - added by seanchayes 22 months ago.
25946.patch (1.1 KB) - added by defries 22 months ago.
25946.10.diff (564 bytes) - added by kovshenin 21 months ago.
25946.11.diff (778 bytes) - added by kovshenin 21 months ago.
25946.12.diff (1.1 KB) - added by kovshenin 21 months ago.
25946.13.diff (586 bytes) - added by kovshenin 21 months ago.
25946.14.diff (7.5 KB) - added by iamtakashi 21 months ago.
25946.15.diff (1.4 KB) - added by iamtakashi 21 months ago.

Download all attachments as: .zip

Change History (56)

@obenland22 months ago

Remove unused class name.

@obenland22 months ago

Simplify and prefix post thumbnail sizes.

comment:1 @lancewillett22 months ago

In 26151:

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

comment:2 @lancewillett22 months ago

In 26152:

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

comment:3 @lancewillett22 months ago

In 26154:

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

@iamtakashi22 months ago

comment:4 @iamtakashi22 months ago

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

@iamtakashi22 months ago

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

@obenland22 months ago

comment:5 @lancewillett22 months ago

In 26217:

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

comment:6 @lancewillett22 months ago

In 26218:

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

comment:7 @lancewillett22 months ago

In 26226:

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

comment:8 @lancewillett22 months ago

In 26227:

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

@iamtakashi22 months ago

comment:9 @iamtakashi22 months ago

Attached a patch.

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

comment:10 @lancewillett22 months 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.

@iamtakashi22 months ago

@iamtakashi22 months ago

comment:11 @iamtakashi22 months 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.

comment:12 @lancewillett22 months 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.

@morganestes22 months ago

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

@morganestes22 months ago

Correct return type from twentyfourteen_font_url().

@morganestes22 months ago

Typo correction.

comment:13 @lancewillett22 months ago

In 26325:

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

comment:14 @lancewillett22 months ago

In 26326:

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

@seanchayes22 months ago

comment:15 @seanchayes22 months 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

comment:16 @lancewillett22 months ago

In 26332:

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

comment:17 @TobiasBg22 months 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.

@defries22 months ago

comment:18 @defries22 months ago

Removing redundant __(.

comment:19 @lancewillett22 months ago

In 26355:

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

comment:20 @chipbennett22 months ago

  • Keywords has-patch added

@kovshenin21 months ago

comment:21 @kovshenin21 months ago

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

@kovshenin21 months ago

comment:22 follow-up: @kovshenin21 months ago

In 25946.11.diff prefix the genericons stylesheet handle.

@kovshenin21 months ago

comment:23 @kovshenin21 months ago

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

@kovshenin21 months ago

comment:24 @kovshenin21 months 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.

comment:25 follow-up: @kovshenin21 months 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?

comment:26 @iamtakashi21 months ago

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

@iamtakashi21 months ago

@iamtakashi21 months ago

comment:27 @iamtakashi21 months ago

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

comment:28 in reply to: ↑ 22 @obenland21 months 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).

comment:29 in reply to: ↑ 25 @obenland21 months 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.

comment:30 @lancewillett21 months 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.

comment:31 @lancewillett21 months ago

In 26561:

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

comment:32 @lancewillett21 months ago

In 26562:

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

comment:33 @lancewillett21 months ago

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

Please open new tickets for new issues.

comment:34 @lancewillett21 months 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.

comment:35 @lancewillett21 months ago

In 26694:

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

comment:36 @lancewillett21 months ago

In 26699:

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

comment:37 @lancewillett21 months ago

In 26810:

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

Note: See TracTickets for help on using tickets.