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

Main menu visible on mobile devices during page load #2536

Closed
thexmanxyz opened this issue Sep 7, 2019 · 12 comments
Closed

Main menu visible on mobile devices during page load #2536

thexmanxyz opened this issue Sep 7, 2019 · 12 comments
Labels
Milestone

Comments

@thexmanxyz
Copy link
Contributor

Hey there,

IDK if this one has already been reported but after @jozwkat pointed out in the Gitter chat room I also remembered that I faced the same issue on my sites and actually never reported it. But I will now finally state the issue.

On mobile devices (e.g. hamburger menu is active and replacing main menu) there might be a short period of time during page load when the actual desktop main menu is rendered before it disappears. If I am remembering right I had that issue on Hydrogen, Helium and AFAIK also on Photon. I'm only certain that it happened on Chrome Android but I think it also happens on desktop browsers when enable device testing or shrinking window. It might also has something todo with rendering speed of the machine.

I also have fixed the issue with the following CSS rules:

@media only all and (max-width:47.938rem) {
    .g-main-nav {
        display: none;
    }
    .g-block-mm  {
        display: none;
    }
}

This might not be the perfect solution but it gives an approach of solving the problem. .g-block-mm might not be necessary but I included it anyway. See the screenshot where this custom class was added.

grafik

@jozwkat
Copy link

jozwkat commented Sep 8, 2019

Great work! The problem is gone.

@thexmanxyz
Copy link
Contributor Author

@jozwkat Glad to hear that :-)

@N8Solutions
Copy link

N8Solutions commented Sep 10, 2019

@thexmanxyz This is something I remember being discussed quite a long time ago, Feb. 2016! It goes to show how long I've been around the Gantry 5 project, LOL!
There was a lengthy discussion about this that included @hennysmafter & @JoomFX.
Unfortunately I'm having a problem going back very far with Gitter but I was able to find a message that I believe to be the first reply from @hennysmafter to @HenriKorhola to the start of the conversation.
2️⃣ February 16, 2016 10:41 PM
@JoomFX then replied here 3️⃣ February 17, 2016 12:57 AM
and the conversation continued from there but I'm not sure we did anything other than adding a code similar to what you have suggested here.

However, it was @JoomFX that first brought this to the attention of @w00fz a month prior to that conversation here 1️⃣ January 8, 2016 12:14 AM

I remember all this because I had saved a solution for it from the discussion back then.
What I have are these 2 code snippets to add to the custom.scss file.

@include breakpoint(mobile-only) {
    .g-main-nav {
        display:none;
    }
}

or this option which was ultimately dependent on a no-js class being added to the framework so it doesn't work. (read, don't use it

@include breakpoint(mobile-only) {
   body .g-main-nav {
        display:none; 
    }
}

body.no-js .g-main-nav {
display:block;
}

The conversation finishes here with @w00fz 👍 February 18, 2016 1:40 AM saying it is a "perfectly fine solution"

EDIT I failed to mention previously that @w00fz also suggested that it would be fine to simply add the built-in Gantry 5 class hidden-phone to the menu particle as referenced here: ✔️ February 18, 2016 1:39 AM

Hopefully you opening this issue it will help others since no ticket was ever opened about it previously and also provide some links for others to refer to in the future to give it a little perspective.

@thexmanxyz
Copy link
Contributor Author

thexmanxyz commented Sep 10, 2019

@thexmanxyz This is something I remember being discussed quite a long time ago, Feb. 2016! It goes to show how long I've been around the Gantry 5 project, LOL!
There was a lengthy discussion about this that included @hennysmafter & @JoomFX.
Unfortunately I'm having a problem going back very far with Gitter but I was able to find a message that I believe to be the first reply from @hennysmafter to @HenriKorhola to the start of the conversation.
2️⃣ February 16, 2016 10:41 PM
@JoomFX then replied here 3️⃣ February 17, 2016 12:57 AM
and the conversation continued from there but I'm not sure we did anything other than adding a code similar to what you have suggested here.

However, it was @JoomFX that first brought this to the attention of @w00fz a month prior to that conversation here 1️⃣ January 8, 2016 12:14 AM

I remember all this because I had saved a solution for it from the discussion back then.
What I have are these 2 code snippets to add to the custom.scss file.

@include breakpoint(mobile-only) {
    .g-main-nav {
        display:none;
    }
}

or this option:

@include breakpoint(mobile-only) {
   body .g-main-nav {
        display:none; 
    }
   body.no-js .g-main-nav {
        display:block; 
    }
}

I hope that helps to put some perspective on this issue.

@N8Solutions @jozwkat That might be an even better solution that should also be directly integrated into Gantry. I had in my mind that my solution proposed above does not consider mobile-only it was also just meant as an example. Additionally that .no-js fix might be very useful for many as well. I suggest your solution over mine to fix this issue until it finds its way to the repository.

Thanks for adding further information to the issue.

@N8Solutions
Copy link

N8Solutions commented Sep 10, 2019

@N8Solutions @jozwkat That might be an even better solution that should also be directly integrated into Gantry. I had in my mind that my solution proposed above does not consider mobile-only it was also just meant as an example. Additionally that .no-js fix might be very useful for many as well. I suggest your solution over mine to fix this issue until it finds its way to the repository.

Thanks for adding further information to the issue.

@thexmanxyz & @jozwkat I edited my reply after I read through the older comments further. The no-js option was never added to the framework. The reason why is explained by @w00fz here ✔️ February 18, 2016 1:39 AM

EDIT I failed to mention previously that @w00fz also suggested that it would be fine to simply add the built-in Gantry 5 class hidden-phone to the menu particle as referenced here: ✔️ February 18, 2016 1:39 AM

@thexmanxyz
Copy link
Contributor Author

thexmanxyz commented Sep 10, 2019

@N8Solutions I thought about an approach to the problem and I ended up with the idea to let the user decide whether he wants take the risk of "no JS support" or prevent flickering. Hence I added a new field to the menu particle which adds a CSS class that disables the menu completely on mobile devices. I will link the respective PR in a bit.

@thexmanxyz
Copy link
Contributor Author

@N8Solutions Referencing back to #2538 please review for more information on my suggested fix.

@mahagr mahagr added this to the 5.4.30 milestone Oct 1, 2019
@mahagr mahagr added the bug label Oct 1, 2019
@mahagr
Copy link
Member

mahagr commented Oct 1, 2019

Merged.

@mahagr
Copy link
Member

mahagr commented Oct 1, 2019

The change was working only in WP, added to other platforms as well.

@thexmanxyz
Copy link
Contributor Author

thexmanxyz commented Oct 2, 2019

@mahagr You are right I overlooked the menu.yaml file located under engines/common/nucleus/particles. Glad that you noticed the missing update on that file! 👍

@N8Solutions
Copy link

N8Solutions commented Feb 25, 2020

@w00fz also suggested that it would be fine to simply add the built-in Gantry 5 class hidden-phone to the menu particle as referenced here: ✔️ February 18, 2016 1:39 AM

Update: As @thexmanxyz pointed out here #2538 (comment), the feature Hide on Mobile is available under the Styles tab of the Gantry 5 backend.

Also of importance, as mentioned here, #2538 (comment), the Hide on Mobile feature is only available with the Hydrogen and Helium themes. So adding the built-in Gantry 5 class hidden-phone to the Menu Particle will be necessary to get the same feature until, and if, it is added to the other templates.

@thexmanxyz
Copy link
Contributor Author

thexmanxyz commented Feb 25, 2020

@N8Solutions The PR #2538 was implemented and AFAIK it is also available on all platforms. It is visible for me on Joomla on all my Gantry 5 installs. Please check #2538 for more information where this settings can be found. Also issue #2574 is about this topic and there are also some screenshots attached showing where this option is located.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants