WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 20 months ago

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

Download all attachments as: .zip

Change History (56)

@obenland21 months ago

Remove unused class name.

@obenland21 months ago

Simplify and prefix post thumbnail sizes.

comment:1 @lancewillett21 months ago

In 26151:

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

comment:2 @lancewillett21 months ago

In 26152:

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

comment:3 @lancewillett21 months ago

In 26154:

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

@iamtakashi21 months ago

comment:4 @iamtakashi21 months ago

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

@iamtakashi21 months ago

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

@obenland21 months ago

comment:5 @lancewillett21 months ago

In 26217:

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

comment:6 @lancewillett21 months ago

In 26218:

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

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

In 26227:

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

@iamtakashi21 months ago

comment:9 @iamtakashi21 months ago

Attached a patch.

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

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

@iamtakashi21 months ago

@iamtakashi21 months ago

comment:11 @iamtakashi21 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 @lancewillett21 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.

@morganestes21 months ago

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

@morganestes21 months ago

Correct return type from twentyfourteen_font_url().

@morganestes21 months ago

Typo correction.

comment:13 @lancewillett21 months ago

In 26325:

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

comment:14 @lancewillett21 months ago

In 26326:

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

@seanchayes21 months ago

comment:15 @seanchayes21 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 @lancewillett21 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 @TobiasBg21 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.

@defries21 months ago

comment:18 @defries21 months ago

Removing redundant __(.

comment:19 @lancewillett21 months ago

In 26355:

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

comment:20 @chipbennett21 months ago

  • Keywords has-patch added

@kovshenin20 months ago

comment:21 @kovshenin20 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.

@kovshenin20 months ago

comment:22 follow-up: @kovshenin20 months ago

In 25946.11.diff prefix the genericons stylesheet handle.

@kovshenin20 months ago

comment:23 @kovshenin20 months ago

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

@kovshenin20 months ago

comment:24 @kovshenin20 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: @kovshenin20 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 @iamtakashi20 months ago

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

@iamtakashi20 months ago

@iamtakashi20 months ago

comment:27 @iamtakashi20 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 @obenland20 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 @obenland20 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 @lancewillett20 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 @lancewillett20 months ago

In 26561:

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

comment:32 @lancewillett20 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 @lancewillett20 months ago

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

Please open new tickets for new issues.

comment:34 @lancewillett20 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 @lancewillett20 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 @lancewillett20 months ago

In 26699:

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

comment:37 @lancewillett20 months ago

In 26810:

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

Note: See TracTickets for help on using tickets.