Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#5889 closed defect (bug) (fixed)

JS in bp_core_register_common_scripts() needs to be enqueued

Reported by: r-a-y Owned by:
Milestone: 2.1.1 Priority: normal
Severity: normal Version:
Component: Templates Keywords: has-patch commit
Cc: markusweser@…, ian.dunn@…

Description

Some BP jQuery scripts are not being loaded when using bp-default.

While the scripts are registered in bp_core_register_common_scripts(), they also need to be enqueued.

See:
http://buddypress.org/support/topic/jquery-bug-in-buddypress-2-1/

Attached patch enqueues these scripts as well to ensure that WP will load them.

Attachments (2)

5889.01.patch (464 bytes) - added by r-a-y 5 years ago.
5889.bp-default.patch (771 bytes) - added by r-a-y 5 years ago.

Download all attachments as: .zip

Change History (24)

@r-a-y
5 years ago

#1 @DJPaul
5 years ago

I don't think this is the right fix. These were moved here explicitly so we can register all our core scripts in one place. If some part of BP needs to use one of these, then wp_enqueue_script should be used in that place, rather than load all of them all of the time.

If BP-Default is not enqueuing its script requirements correctly, the fix needs to be in bp-default, not here.

#2 @r-a-y
5 years ago

Gotcha. Will look into a fix for bp-default.

#3 @r-a-y
5 years ago

bp-default.patch is meant to be applied to the bp-default Github repo.

Had to add the is_active_widget() check for the BP Members widget. I'm loading bp-default as a regular theme from /wp-content/themes/ so I'm guessing it's because of a load order issue.

#4 @boonebgorges
5 years ago

Agreed with DJPaul that bp-defaut seems like a better place to fix this. r-a-y, I have not tested your patch, but it looks right to me. I *think* you should have commit access to bp-default on github.

#5 @r-a-y
5 years ago

I do have commit access to the Github repo, but just wanted a sanity check from someone else.

#6 @m@…
5 years ago

  • Cc markusweser@… added

Please extend 5889.bp-default.patch by

wp_enqueue_script( 'bp-jquery-scroll-to' );

as it's also missing, resulting in another JS error when attempting to comment for instance.

Last edited 5 years ago by m@… (previous) (diff)

#7 @DJPaul
5 years ago

  • Component changed from Core to Theme
  • Keywords commit added

Yep, add scroll-to and this should all be resolved.

I caused these problems when I moved the inline JS scripts out of BP-Default + Classic's global.js file. It's pretty obvious that I neglected to test properly with BP-Default, and that no-one else really did during beta, otherwise we'd likely have caught this. Lesson learnt for the future. :)

#8 @iandunn
5 years ago

  • Cc ian.dunn@… added

#9 @r-a-y
5 years ago

Yep, add scroll-to and this should all be resolved.

I've added the fix on the bp-default Github repo.

I enqueued the scrollTo script only on activity pages, since this is only used on those pages.

It's pretty obvious that I neglected to test properly with BP-Default, and that no-one else really did during beta, otherwise we'd likely have caught this. Lesson learnt for the future. :)

Yeah, I forgot to test bp-default as well! :(

#10 @m@…
5 years ago

Thank you guys. May I permit, in this place, to point you to another important thing missing in bp-default since BP 2.1, and kindly ask you to accept my pull request: https://buddypress.trac.wordpress.org/ticket/5892

#11 @dhuntsha
5 years ago

Can you post the full path name for the file? There are a few files ending in "functions.php".

???buddypress/bp-core/bp-core-functions.php

Thanks.

#12 @r-a-y
5 years ago

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

Can you post the full path name for the file? There are a few files ending in "functions.php".

dhuntsha - Read the first point in:
https://buddypress.org/support/topic/buddypress-2-1-known-issues/

If you have any other questions, please create a new thread in the forums. Thanks!

---

This is pretty much fixed on the bp-default Github repo:
https://github.com/buddypress/BP-Default/commit/619efe8527847266c961fa43775eade42c7ff70d

If there are any more problems with bp-default, please open a new ticket. Thanks everyone!

Last edited 5 years ago by r-a-y (previous) (diff)

#13 @Ypswytch
5 years ago

I installed these patches and it did fix the problem until I logged out. Then the Activity stream would no longer load and the message said "Sorry, there was no activity found. Please try a different filter." Logging back in as the same user did not help. In IE the problem did not not resolve until I closed down the browser, then reopened and went back to the website. But as soon as logged in and then logged back out again the problem would reappear.

By the way, I was having a problem with my "Load More" button not working and this patch fixed that, at least until I could no longer display the activity list.

#14 @Ypswytch
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#15 @r-a-y
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

I installed these patches and it did fix the problem until I logged out.

Thanks for testing, Ypswytch.

I can confirm this issue, but it is not related to this ticket. See the newly-created ticket, #5909, which also has a fix.

#16 @micasuh
5 years ago

I'm confused about this patch, will it be added to the bp_core files? I didn't see anything on the trac or github to reflect the changes in the patches.

#17 @m@…
5 years ago

This was included in BP v2.1.1, hence closed.

#19 @m@…
5 years ago

Oh - I just verified the patch for bp-default theme. Not sure how it's handled for bp-core/bp-core-cssjs.php.

#20 @micasuh
5 years ago

Do both of these patches fix the same problem? If so, is the correct solution to implement the bp-default patch instead of the core file patch?

This ticket was mentioned in Slack in #buddypress by hnla. View the logs.


5 years ago

#22 @DJPaul
3 years ago

  • Component changed from Appearance - Template Parts to Templates
Note: See TracTickets for help on using tickets.