WordPress.org

Make WordPress Core

#21942 closed enhancement (fixed)

Make twentytwelve_frontpage_sidebar_class() pluggable/filterable

Reported by: mrwweb Owned by: lancewillett
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.5
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

I'm making a Twenty Twelve child theme and wanted to add a third column to the sidebar. If using the third column, this plugin would still output the "two" class.

Hence, I think twentytwelve_frontpage_sidebar_class() should be pluggable. Alternately, the $classes array could be filterable, but I think that's more complicated than it needs to be.

I see no reason not to make the function pluggable, but am open to others' thoughts. I'll add a patch later today or tomorrow if no one beats me to it.

Attachments (1)

21942.diff (4.0 KB) - added by lancewillett 19 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 lancewillett19 months ago

  • Milestone changed from Awaiting Review to 3.5

Hi mrwweb:

I'm making a Twenty Twelve child theme and wanted to add a third column to the sidebar.

Can you explain the use case? Do you mean for the front page template?

If for inner pages, why do you need a third column?

Right now in the theme the number class selector is only used for the front page template.

comment:2 lancewillett19 months ago

Discussing with Nacin in IRC #wordpress-dev channel [log https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2012-09-20&sort=asc#m460185] today, looks like we could improve this a bit:

  1. Move to body_class instead
  2. s/twentytwelve_frontpage_sidebar_class/twentytwelve_front_page_sidebar_class
  3. Maybe use a more semantic sidebar class value (I'm open to suggestions — the number seems semantic to me, it's no prescribing any positioning.)

comment:3 lancewillett19 months ago

  • Component changed from Themes to Bundled Theme

comment:4 follow-up: mrwweb19 months ago

Sorry, should've been clearer (just starting to get into core bug reports/patches).

Yes, I only mean on the front page template.

I think the use case is fairly straight forward: I want three columns of widgets grouped vertically. The vertical grouping is what really requires the extra widget area rather than just CSS changes to put three widgets in a row. I went ahead and made the function pluggable on my local copy (that was easy enough) and tweaked the child theme accordingly. Here's the desired result:

http://mrwweb.com/test/twentytwelve/twentytwelve_three_sidebars_front_page.png

I could see this moving to body_class though that would obviously entail additional changes to the stylesheet.

comment:5 in reply to: ↑ 4 lancewillett19 months ago

Replying to mrwweb:

Sorry, should've been clearer (just starting to get into core bug reports/patches).

No worries, I just wanted your perspective to be sure we're on the same page.

Thanks for the info and screenshot, very helpful.

  1. Move to body_class instead
  2. s/twentytwelve_frontpage_sidebar_class/twentytwelve_front_page_sidebar_class
  3. Maybe use a more semantic sidebar class value (I'm open to suggestions — the number seems semantic to me, it's not prescribing any positioning.)

1 and 3 are doable, 2 is mute point of 1.

Last edited 19 months ago by lancewillett (previous) (diff)

lancewillett19 months ago

comment:6 lancewillett19 months ago

  • Keywords has-patch added; needs-patch removed

Patch implements the move to body_class and renames the class to "two-sidebars".

comment:7 follow-up: nacin19 months ago

Seems like there should be a page template check as well? We only want .two-sidebars on the front-page.php template.

comment:8 in reply to: ↑ 7 lancewillett19 months ago

Replying to nacin:

Seems like there should be a page template check as well? We only want .two-sidebars on the front-page.php template.

Correct, and good point.

comment:9 lancewillett19 months ago

#21965 was marked as a duplicate.

Version 0, edited 19 months ago by lancewillett (next)

comment:4 follow-up: lancewillett19 months ago

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

In [21975]:

Twenty Twelve: remove twentytwelve_frontpage_sidebar_class() and make front page sidebar number filterable via body_class. Fixes #21942.

Note: See TracTickets for help on using tickets.