Additional styling support for BuddyPress Widgets

[mercime]

Currently, buddypress.css styles BP widgets on the premise that theme authors are registering their sidebars like the WP default themes do, where variables can be replaced with the necessary hooks like so:

register_sidebar( array(
	...
	'before_widget' => '<aside id="%1$s" class="widget %2$s">',
	...
) );

However, there are theme authors which do not follow that procedure when registering sidebars. Therefore, many BP widgets are not styled properly because the ID's and classes required are not inserted in the markup. For example, the Grisaille theme from the WP theme repo registers sidebars as follows:

register_sidebar(
	array(
		...
		'before_widget' => '<div class="sidebaritem">',
		...
	)
) );

Therefore the selector founds in buddypress.css, for example .widget.buddypress div.item-avatar img.avatar, are not styling BP widgets since the widget and buddypress classes are not generated in the Grisaille theme's widget container.

Attached image shows the unstyled widgets of the Grisaille theme. I included the Twenty Eleven because the disc list style was showing up.

Patch will follow.

[mercime]

Attached patch adds a .bp-widget class to three BP widgets which have unordered lists: Groups, Members and Friends widgets to enable styling of those lists with lower specificity than the original styles in buddypress.css.

Image attached shows screenshots of the Twenty Eleven and Grisaille themes after the patch is applied.

[DJPaul]

The new .item-options and .avatar-block CSS seems too general if you're trying to get the new CSS to only target widget blocks. Did you mean to add the new bp-widget class to these?

I am also not sure about the name of the bp-widget class; to me, that sounds like something I would expect to see on an overall wrapper element, not on ULs in (not all of) the widgets. The duplicated CSS (for backpat) is understandable but kinda annoying.

For a different suggestion, if the root problem is that before_widget isn't guaranteed to contain class=widget, how about we make sure it is? It would be a straightforward change, and this way, the CSS doesn't need to change.

[mercime]

The new .item-options and .avatar-block CSS seems too general if you're trying to get the new CSS to only target widget blocks. Did you mean to add the new bp-widget class to these?

I only targeted the widgets which had no styles using the new bp-widget class: Friends, Groups and Members for a simple but effective fix by removing list style, left margin, and styling content within those three widgets.

Per my check, .avatar-block and .item-options are only used in BP widgets, i.e., not used in any other elements within any BP component, so there are no conflicts.
.avatar-block is used only in the "Who's Online" and "Recently Active" widgets
.item-options is used only in the Friends, Groups, and Members widgets

As for BP Blog Posts widget for multisite installations, there's no issue with having a list style, if any, for the links there.

I am also not sure about the name of the bp-widget class; to me, that sounds like something I would expect to see on an overall wrapper element, not on ULs in (not all of) the widgets. The duplicated CSS (for backpat) is understandable but kinda annoying.

Agreed. Used bp-widget as it's a short but specific class name. Can be changed :)

For a different suggestion, if the root problem is that before_widget isn't guaranteed to contain class=widget, how about we make sure it is? It would be a straightforward change, and this way, the CSS doesn't need to change.

That would be ideal. How do we do it? In addition to class=widget, we also need class=buddypress for styles rendered by following specific selectors like .widget.buddypress div.item-meta or .widget.buddypress ul.item-list img.avatar among others. There are also themes which use 'before_widget' => '', instead.

P.S. This is the second trac ticket I created arising from ticket ​#5804 :)

[DJPaul]

This is probably too late for 2.1 now, but:

...if the root problem is that before_widget isn't guaranteed to contain class=widget, how about we make sure it is?

Something like this ​https://wordpress.stackexchange.com/questions/108400/add-class-to-before-widget-for-all-widgets-with-a-dropdown-and-a-counter

[r-a-y]

This is similar to an issue I brought up in ticket:4869#comment:22:

in the CSS, we make assumptions that every theme will use a 'widget' class.

That is not always the case. For example, TwentyTen uses the 'widget-container' class instead of 'widget'. Therefore, the widget styles will not apply.

The widget JS will also fail because the JS explicitly depends on the 'widget' class

I'm guessing that BP's widget JS also does not work on the Grisaille theme.

Check out the other comments on that ticket for suggested fixes as well.

[mercime]

Thank you DJPaul and r-a-y for your feedback :)

The patch which fixes missing classes needs to address:

• if there is no 'class' attribute in the widget container, then we need to insert 'class="widget buddypress"' via $before_widget
• if there is a 'class' attribute but we're not sure if they even added "widget" to the class and/or the required variable %2$s" to output "widget buddypress" classes, then we still need to add the classes "widget buddypress" via $before_widget to ensure that we have those classes in the widget container.

Working on a patch.

In the meantime, we also need to address the styles of the BP lists in widgets even after we add the necessary classes to the widget. Attached are two CSS patches for consideration:

• 5817-margin-liststyle.patch - addresses only the margins and list styles added by some themes
• 5817-margin-liststyle-avatars.patch - addresses margins, list styles, and makes all avatars in BP widgets the same size: 40px x 40px. Screenshot with style fixes on this patch is also attached (with the patch I'm working on to add before_widget classes).

[r-a-y]

In 9341:

bp-legacy: Improve styling of widgets with avatars.

This removes the default bullet lists from widgets with avatars that might
not be styled in certain WP themes. Commit also enlarges widget avatars
when displayed in a list.

Props mercime.

See #5817.

[r-a-y]

I've committed mercime's CSS changes as they made sense to include.

The last part about adding the widget buddypress classes into our BP widgets for non-standard WP themes is the hang-up. If we were to explore this idea, DJPaul brought up a potential solution in comment:5.

Though, a part of me thinks that the themes that are doing-it-wrong should fix up their widget sidebar calls.

Are we okay with marking this as fixed for 2.2? If we feel that this needs more, let's explore the widget class injection in 2.3.

[r-a-y]

Per ​https://buddypress.org/support/topic/bp-2-2-beta-1-login-avatar/, I've uploaded a patch so the login widget avatar is appropriately sized at 40px x 40px.

The problem was the login widget avatar's width was being resized at 40px, but the height was not. Tested on twentytwelve and twentyfourteen.

[r-a-y]

In 9394:

bp-legacy: Resize avatar in login widget to 40px x 40px.

r9341 had the side-effect of resizing the login widget avatar to 40px x
60px. This commit ensures that this avatar is sized proportionally.

See #5817.

[mercime]

Patches to make sure that the required classes used in buddypress.css are present in the markup of any theme's sidebar widget.

5817-4a.diff attached

• Targets BP widgets only
• Inserts a new div with required classes "widget buddypress" within existing widget container
• Removes possibility of duplicating .widget padding or margin which might have been added to the original .widget container

5817-4b.diff attached

• Targets BP widgets only
• Inserts required classes "widget buddypress" before other classes

Having done the above, I agree with @ray that "that the themes that are doing-it-wrong should fix up their widget sidebar calls". Which is why: 1) a. I also posted a note for the theme author at ​https://themes.trac.wordpress.org/ticket/11170#comment:3 to consider making the changes in his theme even if the last theme update was 2 years ago; and 2) I did not include any support for themes which did not add the class attribute in their theme's $before_widget

[DJPaul]

Let's do the other changes on this for 2.3.

[r-a-y]

Clears the list item. Fixes issues with staggering avatars exhibited in Twenty Fourteen.

[DJPaul]

Ray attached a patch for a bug these 2.2 commits caused in the members widget per ​https://buddypress.org/support/topic/style-of-buddypress-members-widget-broke-after-updating-to-2-2/

Moving to 2.2.1

[johnjamesjacoby]

Patch looks fine to me.

[johnjamesjacoby]

This still looks good. I can commit today if no one beats me to it.

[r-a-y]

In 9473:

bp-legacy: Clear list item in widgets with avatars.

Fixes an issue with staggering avatars exhibited in some themes such as
Twenty Fourteen.

See #5817.

[r-a-y]

In 9474:

bp-legacy: Clear list item in widgets with avatars.

Fixes an issue with staggering avatars exhibited in some themes such as
Twenty Fourteen.

See #5817 (2.2-branch)

[r-a-y]

I've committed the clear list item fix.

The other patches regarding widget CSS injection need to be reviewed and looked at for 2.3.

[mercime]

It is evident that we do need to inject the widget and buddypress classes in before_widget to provide the styling support we provide in buddypress.css for our widgets because we cannot rely on themes to provide the necessary elements to inject the classes for the following reasons:

1. It is not even a requirement in the WordPress Theme Review process to add the class element and the variable in before_widget, i.e., a theme can be approved even without those elements. The ​WP.org Theme Check on widgets used in the theme submission process doesn't check for said elements.

2. Since Matt ​joined in the theme review slack channel, the theme review team has already started researching for the best way for a separate upload process for theme authors to get their themes in the WP theme repository ​without needing to go through a full review by a WP theme reviewer. That separate process will use the Theme Check plugin, probably beefed up a little but doubtful if it will check for the elements we need.

3. Just to note that, while not directly related to this discussion, even the ​WP.com VIP Scanner on widgets does not check for the before_widget items at all, only the register_sidebar location ID.

So at this point in addition to 5817-4b.diff posted above, I suggest removing buddypress widget from the calls to WP_Widget constructor in respective components to remove duplication of classes. Advice on how to proceed with this ticket would be great.

[r-a-y]

mercime - Thanks for the rundown about how Theme Check does not look for dynamic CSS classes.

It may be a good issue to raise here:
​https://github.com/Otto42/theme-check

As for widget CSS class injection, I've tweaked your 5817-4b.diff patch and made it so it only adds the 'widget' and 'buddypress' CSS classes if they do not exist.

I tested this on the Grisaille and Twenty Fifteen theme and it works.

Note: I haven't altered any of our current widgets in the patch because we haven't done anything to them since v1.7 and they still work ;)

Yes, I'm aware that for the twenty-* themes, that the 'widget' class is duplicated due to the classname:
​https://github.com/WordPress/WordPress/blob/master/wp-content/themes/twentyfifteen/functions.php#L136
​https://buddypress.trac.wordpress.org/browser/tags/2.3.2/src/bp-members/bp-members-widgets.php?marks=44,257,407#L41

[mercime]

Thank you @r-a-y. Tested the patch and it works as you mentioned on the Grisaille theme and the WP bundled themes :)

[r-a-y]

mercime - Let's add this. Since you have sheparded this ticket and since you now have commit access, I'll let you do the honors!

[mercime]

@r-a-y Thank you. I was about to follow up on this one :)

[mercime]

Patch has been committed to trunk but erroneously attributed to #6532, my bad. See ​https://buddypress.trac.wordpress.org/ticket/6532#comment:13

Closing this ticket as fixed.