WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 13 days ago

#19709 assigned enhancement

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

Reported by: dgwyer Owned by: christophherr
Milestone: Future Release Priority: normal
Severity: normal Version: 2.2
Component: Widgets Keywords: has-patch good-first-bug needs-unit-tests
Focuses: Cc:

Description

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>'

or

'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 3 years ago.
added new array args 'before_sidebar' and 'after_sidebar'
19709-2.diff (4.7 KB) - added by christophherr 21 months ago.
Refreshes the patch and adds docs

Download all attachments as: .zip

Change History (15)

#1 @c3mdigital
5 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
5 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
5 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
3 years ago

  • Keywords needs-patch added

@flixos90
3 years ago

added new array args 'before_sidebar' and 'after_sidebar'

#5 @flixos90
3 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.


21 months ago

#7 @johnbillion
21 months ago

  • Keywords needs-refresh good-first-bug added

#8 @morganestes
21 months 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
21 months ago

  • Keywords good-first-bug added

@christophherr
21 months ago

Refreshes the patch and adds docs

#10 @christophherr
21 months 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.


21 months ago

#12 @DrewAPicture
12 months ago

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

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

#13 @flixos90
13 days ago

  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release
Note: See TracTickets for help on using tickets.