Skip to:
Content

BuddyPress.org

Opened 10 months ago

Closed 10 months ago

Last modified 10 months ago

#9033 closed defect (bug) (fixed)

Breaks twentyfifteen theme menu

Reported by: shawfactor's profile shawfactor Owned by: imath's profile imath
Milestone: 12.1.0 Priority: normal
Severity: normal Version:
Component: Templates Keywords: has-patch commit
Cc: emaralive

Description

The update moves the no-js class to the body tag which breaks twentyfifteen theme menu which has it on the html tag.

Attachments (1)

buddypress-no-js-2015.png​ (52.2 KB) - added by sabernhardt 10 months ago.
BuddyPress Legacy 12.0.0 has .no-js body class while Twenty Fifteen replaces its HTML class with js

Download all attachments as: .zip

Change History (15)

#1 @imath
10 months ago

  • Keywords reporter-feedback added

Hi thanks for your report. I’m a bit amazed about it because I believe it was probably the case in previous versions of BuddyPress. Can you share with us the active template pack on your site ?

You can easily find this information from Tools > Site Health. Activate the debug info tab and open the BuddyPress panel, which should be in last position.

#2 @sabernhardt
10 months ago

  • Keywords reporter-feedback removed
  • Severity changed from normal to major

I found the no-js body class on the home page, with both BuddyPress Legacy and BuddyPress Nouveau 12.0.0, when JavaScript was enabled.

Version 11.1.0 had printed a ​special script that switched the class to `js`, which is no longer available. Similar code is in ​the `bp-nouveau` script, but somehow my site does not enqueue that script (at least on the front page).

I noticed the body class in Twenty Fourteen, Twenty Fifteen, Twenty Twenty, Twenty Twenty-Two, Twenty Twenty-Three and Twenty Twenty-Four. However, ​Twenty Twenty-One specifically removes it from the body element.

@sabernhardt
10 months ago

BuddyPress Legacy 12.0.0 has .no-js body class while Twenty Fifteen replaces its HTML class with js

#3 @sabernhardt
10 months ago

Similarly with the BP Legacy template, I do not have the bp-legacy/js/buddypress.js script enqueued. That file also has a ​replacement script.

#4 @imath
10 months ago

  • Component changed from Core to Templates
  • Milestone changed from Awaiting Review to 12.1.0
  • Severity changed from major to normal

Hi @sabernhardt

Thanks a lot for your feedback. Thanks to it I believe I know where it comes from.

Can you try to use this:

add_filter( 'bp_enqueue_assets_in_bp_pages_only', '__return_false' );

This ticket was mentioned in ​PR #201 on ​buddypress/buddypress by ​@imath.
10 months ago #5

  • Keywords has-patch added

​r13418 introduced a regression leaving the no-js body class when not in the community area as we do not load community assets everywhere on the site anymore and some themes are using this class to be informed whether JavaScript is enabled or not (eg: TwentyFifteen).

We forgot we also needed to avoid adding this no-js body class when not in a community area (See #8679).

Trac ticket: ​https://buddypress.trac.wordpress.org/ticket/9033

#6 @sabernhardt
10 months ago

With the filter, assets loaded on my home page. Therefore the no-js class was added and switched to js with JavaScript enabled.

I also tried copying and pasting the new code from the PR to each template functions file. That removed the no-js class from the home page.

If someone else could test the pull request more thoroughly, that would be good.

#7 @imath
10 months ago

  • Keywords needs-testing added

Thanks for your feedback @sabernhardt 👍

@emaralive or @shawfactor can you give a test to the PR ?

#8 @emaralive
10 months ago

  • Cc emaralive added

@imath

Theme: TwentyFifteen

When not in community area, e.g, blog page/post, privacy policy

Without PR 201 (patch/9033) - body class:

Active template pack: BuddyPress Nouveau
bp-nouveau post-template-default single single-post postid-1 single-format-standard logged-in admin-bar wp-embed-responsive no-js customize-support

Active template pack: BuddyPress Legacy
bp-legacy post-template-default single single-post postid-1 single-format-standard logged-in admin-bar wp-embed-responsive no-js customize-support

With PR 201 (patch/9033) - body class:

Active template pack: BuddyPress Nouveau
bp-nouveau post-template-default single single-post postid-1 single-format-standard logged-in admin-bar wp-embed-responsive customize-support

Active template pack: BuddyPress Legacy
bp-legacy post-template-default single single-post postid-1 single-format-standard logged-in admin-bar wp-embed-responsive customize-support

#9 @imath
10 months ago

  • Keywords commit added; needs-testing removed

Thanks a lot @emaralive 👍

I’ll commit the fix shortly.

#10 @shawfactor
10 months ago

This is occurring on twentysixteen as well btw.I've just monkey patched it by making the css rules more specific (and raised it with them)

But there probably should be a stanadrda place to add no-js. Either that or buddyress should namespace its no-js classes e.g. <body class="bp-no-js">

#11 @imath
10 months ago

@shawfactor

Thanks for your feedback. I agree about namespacing the classes.

That being said, we’ve communicated for a year about the BP Rewrites API change hoping 3rd party plugins would update their code and a majority didn’t. So I can imagine namespacing classes right away would probably generate other troubles.

We’ll simply fix the no-js regression for now as we’re preparing a minor release (12.1.0).

#12 @imath
10 months ago

In 13672:

BP Template packs: only add the no-js body class in community area

Since [13418] we do not load community assets by default everywhere on the site anymore.

But this commit appears to be incomplete: we also need to avoid adding the no-js body class when not in a community area.

NB: returning false to the 'bp_enqueue_assets_in_bp_pages_only' filter will carry on loading BP Template pack assets everywhere on the site.

Props sabernhardt, emaralive, shawfactor

See #8679
See #9033 (trunk)
Closes ​https://github.com/buddypress/buddypress/pull/201

#13 @imath
10 months ago

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

In 13673:

BP Template packs: only add the no-js body class in community area

Since [13418] we do not load community assets by default everywhere on the site anymore.

But this commit appears to be incomplete: we also need to avoid adding the no-js body class when not in a community area.

NB: returning false to the 'bp_enqueue_assets_in_bp_pages_only' filter will carry on loading BP Template pack assets everywhere on the site.

Props sabernhardt, emaralive, shawfactor

See #8679
Fixes #9033 (branch 12.0)

This ticket was mentioned in ​Slack in #buddypress by imath. ​View the logs.
10 months ago

Note: See TracTickets for help on using tickets.