Make WordPress Core

Opened 10 years ago

Last modified 6 years ago

#29790 new enhancement

Widgets don't know the widget area context they're in

Reported by: ruudjoyo's profile ruud@… Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Widgets Keywords: dev-feedback needs-patch
Focuses: administration Cc:

Description

If you have a widget in a widget area (both on the admin side and front-end side) it does not know in which widget area it is in.

Use cases for this would be:

  • custom filtering and/or styling on the front-end of a widget based on its location.
  • having some editor options turned on/off based on widget-area.

Attachments (1)

29790.patch (1.5 KB) - added by ruud@… 10 years ago.
Patch to add widget area information to widget instance

Download all attachments as: .zip

Change History (17)

#1 @ruud@…
10 years ago

Related ticket: #14686

Possible solution: add widget area arguments/data into the widget instance would allow widget developers to use it in their widget code.

@ruud@…
10 years ago

Patch to add widget area information to widget instance

#2 @ruud@…
10 years ago

  • Keywords has-patch dev-feedback added

#3 @anonymized_14009747
10 years ago

Just checked this; patch adds an array with the arguments including the id of the current sidebar it is in.
Also successfully tested in the theme customizer.

#4 follow-up: @westonruter
10 years ago

I think it would be better to add the widget ID to the $args passed into the callback instead of adding it to the $instance which is supposed to reflect the stored database value.

#5 in reply to: ↑ 4 @ruud@…
10 years ago

Replying to westonruter:

I think it would be better to add the widget ID to the $args passed into the callback instead of adding it to the $instance which is supposed to reflect the stored database value.

Hi Weston, thanks for commenting on this ticket.

I too was contemplating if $instance is the proper place for storing this data. Another choice is to use the widget Object itself ($this). Neither place seemed fitting so I opted for the $instance. The thing being that its current sidebar location does say something about this particular instance..

As far as I know those are the two places you I would look for this sort of data if I want to use it from the widget display view and admin view.

What do you mean with adding the widget ID to the $args?
How does that help the widget determine in which sidebar the widget is currently in?

#6 follow-up: @ruud@…
10 years ago

Another thought: how about a widget being able to request its current sidebar via a new function?

#7 in reply to: ↑ 6 @westonruter
10 years ago

I was confused when I first responded to this, so please disregard my comment about $args.

Replying to ruud@…:

Another thought: how about a widget being able to request its current sidebar via a new function?

You can actually find out which sidebar a widget belongs to by adding the following to a widget's update, form, or widget callbacks (any method of the class extending WP_Widget):

$active_sidebar_id = null;
foreach( wp_get_sidebars_widgets() as $sidebar_id => $widgets_ids ) {
        if ( in_array( $this->id, $widgets_ids ) ) {
                $active_sidebar_id = $sidebar_id;
                break;
        }
}

Beware of using this in the admin, however, since widgets can be dragged around to other widget areas.

#8 follow-up: @ruud@…
10 years ago

Hi Weston. Your suggestion is valid indeed.
I know we can leverage the widget callbacks to obtain the widget area information, that's how we currently implemented it for our use case.

However, I think this is perhaps not common knowledge and adding some additional information to the $instance seems rather harmless and opens up possibilities in an easy way for widget developers.

So to get more to the point on this: is it your opinion that because a developer can achieve this via alternative routes it should not go into core? Or is the way (by adding it to the $instance variable) not right?
Don't get this the wrong way, I just want your opinion on this and I personally don't know what should or shouldn't go into core, and on what grounds we can make a decision on this as well.
I mean: should we focus on light weight code, or on ease of use, or on ??

Thanks.

#9 @DrewAPicture
10 years ago

  • Summary changed from Widgets don't know the widget area context there in to Widgets don't know the widget area context they're in

#10 in reply to: ↑ 8 ; follow-up: @westonruter
10 years ago

Replying to ruud@…:

I mean: should we focus on light weight code, or on ease of use, or on ??

Yeah, it's a good question. I think that the logic shared above would make sense as a new method on the WP_Widget class itself. This would make more sense than adding it to the $instance I think. So the widget callbacks could do just do something like $this->get_sidebar_id().

#11 in reply to: ↑ 10 @DrewAPicture
10 years ago

Replying to westonruter:

Replying to ruud@…:

I mean: should we focus on light weight code, or on ease of use, or on ??

Yeah, it's a good question. I think that the logic shared above would make sense as a new method on the WP_Widget class itself. This would make more sense than adding it to the $instance I think. So the widget callbacks could do just do something like $this->get_sidebar_id().

+1 for the method method

#12 @ruud@…
10 years ago

  • Keywords needs-patch added; has-patch removed

I can follow this logic too. Besides a get method, the widget Class should also get a set method right?

I'm thinking a method name something along the lines of:
get_sidebar_context();
set_sidebar_context();
both using an array of information (some array as the $args array from the patch), in that way any known sidebar context information can be stored and retrieved from the widget object.

Furthermore the comments with these functions could(should?) reflect the fact the information may have been altered by dragging the widget from one sidebar into another (in the admin).

I'm not exactly sure how the admin-ajax methods come into play here.

#13 follow-up: @ruud@…
10 years ago

How about dropping 'sidebar' in favor of 'widget-area'? What's the general verdict on this?

get_widget_area_context();
set_widget_area_context();

#14 in reply to: ↑ 13 @westonruter
10 years ago

Replying to ruud@…:

How about dropping 'sidebar' in favor of 'widget-area'? What's the general verdict on this?

get_widget_area_context();
set_widget_area_context();

While “sidebar” has been replaced with “widget area” in the user-facing admin pages, the term “sidebar” is still used exclusively in the WordPress API, so I think it should be used here too.

class WP_Widget {
        /* ... */
        public function get_sidebar_id() {
                foreach ( wp_get_sidebars_widgets() as $sidebar_id => $widgets_ids ) {
                        if ( in_array( $this->id, $widgets_ids ) ) {
                                return $sidebar_id;
                        }
                }
                return null;
        }
Last edited 10 years ago by westonruter (previous) (diff)

#15 @boonebgorges
10 years ago

  • Version trunk deleted

#16 @joedolson
10 years ago

  • Focuses accessibility removed
Note: See TracTickets for help on using tickets.