Skip to content
This repository has been archived by the owner on Sep 16, 2019. It is now read-only.

Problems Mobile and Top Bar in FP6 #598

Closed
arielnoname opened this issue Dec 9, 2015 · 6 comments
Closed

Problems Mobile and Top Bar in FP6 #598

arielnoname opened this issue Dec 9, 2015 · 6 comments
Labels

Comments

@arielnoname
Copy link

Check gif please.

foundationpress

@joshrathke
Copy link
Contributor

Have you changed anything with relation to the Mobile Off-Canvas Menu? Mine seems to work fine.

UPDATE: I thought you referring to the margins being funky. I didn't even notice that the topbar menu was showing up as well. This is happening on mine, but I didn't notice it until now.

@arielnoname
Copy link
Author

Clean install.
Going to try again if I can reproduce on a new clean install.

@joshrathke
Copy link
Contributor

I just noticed that the option that enables the user to choose between "topbar" and "offcanvas" as a means of Navigation doesn't take effect until after a user does an initial save in the "Customizer". This may need more documentation as most developers probably don't even know they need to select a setting.

By default it is supposed to be "topbar" mobile navigation. But I don't think it actually saves it until the user opens up the customizer, navigates to the mobile menu section and "saves". After the initial save, it seems to work just fine.

This is the code in the header that pulls in the "topbar" menu. Which is your "Mystery" menu to the right in your picture above. As you can see it's passing the test that says "if the option isn't set, set to true" or "if the option is set to topbar set to true".

<?php if ( ! get_theme_mod( 'wpt_mobile_menu_layout' ) || get_theme_mod( 'wpt_mobile_menu_layout' ) == 'topbar' ) : ?>
    <?php get_template_part( 'parts/mobile-top-bar' ); ?>
<?php endif; ?>

The problem is that both the topbar and offcanvas check for the setting to "not" be set. So if the user has'nt set the option, or saved it. Both will pass the test, and both will be output to the screen, as shown in the image above.

I propose that since we want to set the "topbar" as the default, we actually remove the "if the setting isn't set" statement from the "Offcanvas" menu so that only if the user has specifically set it to "offcanvas" the offcanvas section shows up. That would solve this problem, and drive Devs to venture into the customizer to make the initial settings save.

<?php if ( get_theme_mod( 'wpt_mobile_menu_layout' ) == 'offcanvas' ) : ?>
     <div class="off-canvas-wrapper">
          <div class="off-canvas-wrapper-inner" data-off-canvas-wrapper>
      <?php get_template_part( 'parts/mobile-off-canvas' ); ?>
<?php endif; ?>

@olefredrik what do you think about this solution? If it's good I can make a PR.

@arielnoname
Copy link
Author

Haven't checked code-wise it yet, so correct me if I'm wrong.

In FP5 the thing was like this:
1.-
fp5-1
2.-
fp5-2

And in FP6 this is it:
1.- fp6-1

2.- My first pic that shows that this doesn't work

So. I think that having those Menus for Right and Left Topbar AND the Mobile tick boxes, was a very big win.

Ps. In the first Picture, the dropdown corresponds to a Topbar element.
Please tell me if you need some assistance with the spanish that's in the GIF. I think you won't need any.

Cheers

@olefredrik olefredrik added the bug label Dec 12, 2015
@olefredrik
Copy link
Owner

Thanks for pointing out the error @josh-rathke . A pull request in on the way.

@arielnoname : I could not get the "left" and "right" options for the offcanvas to work as I wanted before the F6 launch, so I decided to remove it from the theme customizer.

In the menu settings, you can select where your menus will display (topbar and/or mobile)

menu-settings

In the theme customizer you can select a topbar mobile menu or offcanvas mobile menu

customizer

@olefredrik
Copy link
Owner

Fixed #611

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

No branches or pull requests

3 participants