Make WordPress Core

Opened 9 years ago

Last modified 5 days ago

#19709 accepted enhancement

Add 'before_sidebar' and 'after_sidebar' attributes to register_sidebar()

Reported by: dgwyer Owned by: audrasjb
Milestone: 5.6 Priority: normal
Severity: normal Version: 2.2
Component: Widgets Keywords: has-patch good-first-bug has-unit-tests commit
Focuses: Cc:


Whilst experimenting with some code recently I needed to have each widget area wrapped inside a container with the widget id as the container CSS class/id.

If used in a theme this widget area container usually has to be hard coded. This is the case for Twenty Ten, and Twenty Eleven.

It occurred to me that it would be very useful to have an extra couple of attributes available in register_sidebar() such as 'before_sidebar' and 'after_sidebar' so that developers have a consistent way to automatically add a wrapper to each widget area.

It would be additionally useful if you could do something like this to add the widget area id as the wrapper CSS class/id as required:

'before_sidebar' => '<div class="%3$s">'
'after_sidebar' => '</div>'


'before_sidebar' => '<div id="%3$s">'
'after_sidebar' => '</div>'

If this feature is considered then I'm not sure whether 'before_sidebar' or 'before_widget_area' is a better choice for the attribute name.

Attachments (2)

19709.diff (1.3 KB) - added by flixos90 5 years ago.
added new array args 'before_sidebar' and 'after_sidebar'
19709-2.diff (4.7 KB) - added by christophherr 4 years ago.
Refreshes the patch and adds docs

Download all attachments as: .zip

Change History (18)

#1 @c3mdigital
7 years ago

  • Resolution set to wontfix
  • Status changed from new to closed
  • Version changed from 3.3 to 2.2

My argument against this is that child themes would lose some flexibility styling sidebars. Currently they can override sidebar.php in a child theme and add any container wrapper attributes they want. This would make it very hard for a child theme to change the wrapper html. I'm closing as wont fix but if anyone has additional input or a case for this please reopen and it can be discussed further.

#2 @markoheijnen
7 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

I disagree on your argument that you will lose flexibility. It will add flexibility when we add a filter to register_sidebar(). So that way you don't need to overwrite it with a file and maybe even break a child theme when you update your parent theme.

#3 @dgwyer
7 years ago

The idea is to increase flexibility. The 'before_sidebar' and 'after_sidebar' fields would be optional so if left empty (default setting) parent/child themes could still define the sidebar wrapper tags explicitly in theme templates files.

#4 @chriscct7
5 years ago

  • Keywords needs-patch added

5 years ago

added new array args 'before_sidebar' and 'after_sidebar'

#5 @flixos90
5 years ago

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

The patch above adds the two new arguments before_sidebar and after_sidebar into the array. before_sidebar can be a string like <div id="%1$s" class="%2$s"> and will receive sidebar ID and class. A thing to consider is what happens if the class argument is empty (default)? There would be an empty class attribute. I guess in this case the only way to work around it would be to remove the class attribute from the string if it is there.

However I would say that is the theme author's business. What I just described above would probably be too much here.

If the new arguments are provided, they are outputted after the dynamic_sidebar_before action / before the dynamic_sidebar_after action, respectively.

This ticket was mentioned in Slack in #docs by morganestes. View the logs.

4 years ago

#7 @johnbillion
4 years ago

  • Keywords needs-refresh good-first-bug added

#8 @morganestes
4 years ago

  • Keywords good-first-bug removed

register_sidebar() is now in src\wp-includes\widgets.php, and needs the new args added to the doc block.

#9 @morganestes
4 years ago

  • Keywords good-first-bug added

4 years ago

Refreshes the patch and adds docs

#10 @christophherr
4 years ago

  • Keywords needs-docs needs-refresh removed

Attached a refreshed patch and added docs

This ticket was mentioned in Slack in #docs by christophherr. View the logs.

4 years ago

#12 @DrewAPicture
3 years ago

  • Owner set to christophherr
  • Status changed from reopened to assigned

Assigning ownership to mark the good-first-bug as "claimed".

#13 @flixos90
2 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release

This ticket was mentioned in PR #361 on WordPress/wordpress-develop by deepaklalwani97.

3 months ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Added unit test cases and fixed alignment issues from the previous patch.

Trac ticket: https://core.trac.wordpress.org/ticket/19709

#15 @audrasjb
2 weeks ago

  • Milestone changed from Future Release to 5.6
  • Owner changed from christophherr to audrasjb
  • Status changed from assigned to accepted

PR 361 still applies cleanly and in my quick testing, it looks pretty good to me.

Let's try to commit to fix this old ticket in milestone 5.6.

#16 @audrasjb
5 days ago

  • Keywords commit added

Just checked and PR 361 still applies cleanly.
Now marking this for commit.

Note: See TracTickets for help on using tickets.