#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)
Change History (24)
#3
@
10 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
@
10 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
@
10 years ago
I do have commit access to the Github repo, but just wanted a sanity check from someone else.
#6
@
10 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.
#7
@
10 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. :)
#9
@
10 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
@
10 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
@
10 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
@
10 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!
#13
@
10 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.
#15
@
10 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
@
10 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.
#18
@
10 years ago
Please forgive me if I'm missing something but I do not see it in 2.1.1
https://buddypress.trac.wordpress.org/browser/tags/2.1.1/src/bp-core/bp-core-cssjs.php
https://github.com/buddypress/BuddyPress/blob/master/src/bp-core/bp-core-cssjs.php
#19
@
10 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
@
10 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?
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.