Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Separate cache for mobile devices when JetMenu is used #5063

Closed
2 of 6 tasks
DahmaniAdame opened this issue May 20, 2022 · 5 comments · Fixed by #5072
Closed
2 of 6 tasks

Enable Separate cache for mobile devices when JetMenu is used #5063

DahmaniAdame opened this issue May 20, 2022 · 5 comments · Fixed by #5072
Labels
3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting module: cache priority: low Issues that can wait type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Milestone

Comments

@DahmaniAdame
Copy link
Contributor

Before submitting an issue please check that you’ve completed the following steps:

  • Made sure you’re on the latest version 3.11.2
  • Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug
JetMenu recommends using the Separate cache files for mobile devices to avoid responsive issues for their Megamenu.

https://crocoblock.com/troubleshooting/articles/cannot-make-jetmenu-mega-menu-mobile-responsive/

This can be confirmed by diffchecking mobile vs. desktop.

image

To prevent the issue, we should add:

jet-menu/jet-menu.php

to:

public static function get_mobile_plugins() {
return [
'jetpack/jetpack.php' => [
'is_active_callback' => function() {
if ( ! class_exists( 'Jetpack' ) ) {
return false;
}
return \Jetpack::is_active() && \Jetpack::is_module_active( 'minileven' );
},
'activation_hook' => 'jetpack_activate_module_minileven',
'deactivation_hook' => 'jetpack_deactivate_module_minileven',
],
'wptouch/wptouch.php' => [],
'wiziapp-create-your-own-native-iphone-app/wiziapp.php' => [],
'wordpress-mobile-pack/wordpress-mobile-pack.php' => [],
'wp-mobilizer/wp-mobilizer.php' => [],
'wp-mobile-edition/wp-mobile-edition.php' => [],
'device-theme-switcher/dts_controller.php' => [],
'wp-mobile-detect/wp-mobile-detect.php' => [],
'easy-social-share-buttons3/easy-social-share-buttons3.php' => [],
];
}

To Reproduce

  1. Enable JetMenu
  2. Check the difference in markup between mobile and desktop

Expected behavior
JetMenu should be cached properly.

Screenshots
Added to the description.

Additional context
Ticket - https://secure.helpscout.net/conversation/1888674702/343703/

Backlog Grooming (for WP Media dev team use only)

  • Reproduce the problem
  • Identify the root cause
  • Scope a solution
  • Estimate the effort
@mehedihasanziku
Copy link

mehedihasanziku commented May 20, 2022

Note: I did test before, separate cache will not cover all cases.

In one case i need to set separate cache as tablet instead of mobile WP Rocket | Set Tablets As Mobile

Extra Info: separate cache can be avoided if user decided to duplicate that menu for mobile and desktop

@DahmaniAdame
Copy link
Contributor Author

@mehedihasanziku thank you for your feedback.

In one case i need to set separate cache as tablet instead of mobile WP Rocket | Set Tablets As Mobile

This is more of an edge case and is likely related to the responsive breakpoints set by the theme and how the menu fits in the layout.

Extra Info: separate cache can be avoided if user decided to duplicate that menu for mobile and desktop

That would be an action/decision required from the user. We try to make things work out of the box as much as possible.

@piotrbak piotrbak added type: enhancement Improvements that slightly enhance existing functionality and are fast to implement 3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting module: cache priority: low Issues that can wait labels May 22, 2022
@DahmaniAdame DahmaniAdame linked a pull request May 24, 2022 that will close this issue
@Tabrisrp Tabrisrp added this to the 3.11.4 milestone May 31, 2022
@vmanthos
Copy link
Contributor

@DahmaniAdame Do you happen to have a copy of the JetMenu plugin? I need to use that for QA.

@DahmaniAdame
Copy link
Contributor Author

@vmanthos no, I don't. But I can ask the customer if they can provide the plugin for testing.

@vmanthos
Copy link
Contributor

I can ask the customer if they can provide the plugin for testing.

I "fake-installed" JetMenu by creating an empty jet-menu/jet-menu.php plugin. As long as that's the correct file for the plugin we should be good.

But it would be best to test this with the actual plugin. If you don't mind, please ask for it.

@piotrbak piotrbak removed this from the 3.11.4 milestone Jun 16, 2022
mostafa-hisham pushed a commit that referenced this issue Jun 20, 2022
* Force separate mobile cache for JetMenu

Ticket - https://secure.helpscout.net/conversation/1888674702/343703/

* Fixed code standards

Co-authored-by: COQUARD Cyrille <cyrille@wp-media.me>
@piotrbak piotrbak added this to the 3.11.4 milestone Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting module: cache priority: low Issues that can wait type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants