#7314 closed defect (bug) (fixed)
Buddypress issue with Twenty Sixteen menu
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 2.7.3 | Priority: | normal |
| Severity: | normal | Version: | 2.7 |
| Component: | Templates | Keywords: | dev-feedback commit |
| Cc: | salvatore.noschese@… |
Description
I've report here some months ago:
https://buddypress.org/support/topic/issue-with-sixteen-mobile-menu-and-buddypress/
and here (with a provisory fix):
https://wordpress.org/support/topic/sixteen-menu-is-open-on-pageload/?replies=6#post-8575130
I've try again just now with new install in local webserver:
1) download wordpress (ita) and put in wamp directory;
2) run wamp, open localhost, open localhost/wordpress and install (new database etc);
3) install buddypress;
4) setup some item in twenty sixteen menu and test!
issue is present. no problem with others wordpress default theme (so, is just an issue with sixteen).
here a provisory fix, but need more investigation:
https://wordpress.org/support/topic/sixteen-menu-is-open-on-pageload/#post-8371032
hope this info can help (and sorry for poor english).
regards,
S.N.
Attachments (5)
Change History (30)
#1
@
8 years ago
- Cc salvatore.noschese@… added
- Component changed from Core to Templates
just to ble more clear: i've this issue only in mobile mode (then, also if i reduce firefox screen) and is really "unpleasant" with mobile device :/
regads,
S.N.
#2
@
8 years ago
- Keywords reporter-feedback added
@DarkWolf What is the *actual* problem? You've described how to test well, but when I do so, I can't see what's wrong. How is it behaving that you think it shouldn't?
#3
@
8 years ago
The problem is:
With buddypress, during pageload, menu - in mobile mode view - is expanded and turn back ok (just a button) only when pageload is complete.
Without buddypress, instead, menu is never expandend (nor during pageload, nor when pageload is complete) and work as expected (is open only onclick).
Thanks for reply.
Regards,
S.N.
#4
@
8 years ago
- Keywords dev-feedback added; reporter-feedback removed
@DarkWolf notes that the problem is with the addition of the no-js <body> CSS class for bp-legacy:
https://wordpress.org/support/topic/sixteen-menu-is-open-on-pageload/#post-7516921
This conflicts with Twenty Sixteen, which also has its own no-js detection method:
https://github.com/WordPress/twentysixteen/issues/408
Not sure what we should do here. Do we do a conditional to see if Twenty Sixteen is loaded and if so, do not add our custom no-js CSS class?
#5
@
8 years ago
@r-a-y if we start doing checks like this - and I've thought often of doing so - we accept a increasing burden on fine tuning for given themes, if we do so here how many other instances of things that we could manipulate checking what theme is selected and changing code/rules to accommodate do we start doing? ( not saying we don't though).
At this moment checking and I can't replicate this issue though! Browser specific?
Also 2016 isn't the only theme to hardcode the class to the html element 2015 does too.
This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.
8 years ago
#7
@
8 years ago
I've test with Firefox and chrome + Lumia device (iemobile) and issue is present. Then, is an issue with no-js from buddypress with a standard wp theme and, imho, this need,to be fixed (if xyz* cause issue with default wp theme... xyz* need to be fixed).
I've this issue, in my dev live website: laltroweb.it/_dev
+ in localhost clean install (just buddypress and WordPress with some element in menu).
Disable buddypress or remove no-js class from buddypress and all is good.
Clearly, this class cause an issue with default WordPress theme (and I really hope this will be considered a bug - otherwise, sorry, this bug report is unuseful).
Regards,
S.N.
#8
@
8 years ago
Well it wouldn't hurt to have a check available in buddypress-functions.php or core somewhere for that matter for active wp theme.
As for removing classes, my only worry is if styles had been hooked to it.
2015, 2016, 2017 all have this class on the html element fwiw.
@r-a-y lets add a check then? my crude approach $this->wp_theme = strtolower( str_replace( ' ', '', wp_get_theme() ) ); and I guess an array of twenty* themes with that class to check against.
#10
@
8 years ago
I think that is the wrong approach.
--> 2017 has no-js in its template, and removes it if JS is present in its twentyseventeen_javascript_detection() (hooked to wp_head at priority 0), which will run pretty soon after page load. It's VERY near the top of the page.
--> BP adds no-js via body_class and removes it in our buddypress.js, inside a jQuery ready callback. (i.e. much later than 2017's).
I don't understand really what the problem is. Even if the theme temporarily thought it had no JS support, it ought to "flash back" with JS features appearing, after DOM Ready... right? I don't understand why this isn't happening. I'm going to assume it's because of some quirk of load order and timing. (confused!)
At any rate, we could basically just copy/paste the JS in twentyseventeen_javascript_detection() into the VERY top of our buddypress.js, which means that our no-js will be removed much quicker than it is. Can someone who knows how to recreate this problem, test the approach? Or maybe talk me through it on Slack sometime so I can learn.
This ticket was mentioned in Slack in #buddypress by hnla. View the logs.
8 years ago
#12
@
8 years ago
Can we test this workaround css patch.
it attempts to hide the siteheader menu if html.js and body.no-js on the basis that we cut in later with our remove .no-js so onclick of a link and page load the effective state of the body class should be no-js and thse new rules take effect to hide the elements until full page load.
It feels as though it works or at least improves things at a throttled mobile view but equally could be mis-reading result and the rule will not be loaded until we hit the body element so likely isn't really an improvement:(
#17
@
8 years ago
Update patch to include comments for scss/css files ruleset - detail issue for scss, brief note for css.
#20
@
8 years ago
@DarkWolf We have looked at this from a variety of angles to try and see what potential fixes would work, our big problem is in the point at which our scripts cut in to change the class tokens & this isn't an issue we can easily resolve.
We have settled on an approach to reduce the issue by ensuring that the menu elements are hidden if 2016 class is .js (thus JS active) and our body class reads as .no-js ( our script hasn't yet been able to run to change to .js this should help to reduce the noticeable effect on these slower connections.
#21
@
8 years ago
ok, thank'u, i'll test as soon as possible your css patch and release a feedback
regards :)
#22
@
8 years ago
edit: is necessary to edit also twentysixteen.min.css
search:
@charset "UTF-8";
replace:
@charset "UTF-8";@media screen and (max-width:905px){html.js body.no-js .site-header-menu{display:none}}
after of this, all seems to work as expected :)
menu issue during pageload