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

Menu icon in mobile view jumps open->close #3

Open
heykapil opened this issue Mar 16, 2022 · 12 comments · Fixed by #4
Open

Menu icon in mobile view jumps open->close #3

heykapil opened this issue Mar 16, 2022 · 12 comments · Fixed by #4

Comments

@heykapil
Copy link

No description provided.

@vsoch
Copy link
Owner

vsoch commented Mar 16, 2022

Yep reproduced! The trick will be comparing with https://fly.io/docs/ and figuring out what I stripped out that removed the functionality. I do remember testing it early on (and I think it worked) so likely I fussed something up.

@heykapil
Copy link
Author

heykapil commented Mar 16, 2022

Our page is missing essential javascript functionality as https://fly.io/docs/ has. This minified javascript file has our required menu toggle, I'm talking about https://fly.io/public/javascripts/docs.js , but it includes several other things like js for algolia search and all.

@vsoch
Copy link
Owner

vsoch commented Mar 16, 2022

Thanks that's helpful! I must have removed it and not checked all functionality. I'm at work but will put in a PR later this evening to fix it up.

@vsoch
Copy link
Owner

vsoch commented Mar 16, 2022

okay @heykapil please test! #4

@heykapil
Copy link
Author

Not working. Thanks for trying out ! I have removed extra code from https://fly.io/public/javascripts/docs.js to this https://imkapil.netlify.app/assets/js/menu.js
This worked for me. Please see it in action from any mobile device. https://imkapil.netlify.app

@vsoch
Copy link
Owner

vsoch commented Mar 17, 2022

Thats's weird, because it worked locally for me. Could you please open a PR with your fix for me to test?

@vsoch
Copy link
Owner

vsoch commented Mar 17, 2022

Also it works fine in my preview.... https://9-447414597-gh.circle-artifacts.com/0/tw-jekyll/docs/index.html

screen shot after I turned into mobile and clicked the menu. Are you sure you tested the right thing?

image

@heykapil
Copy link
Author

Yup, menu is working in this https://9-447414597-gh.circle-artifacts.com/0/tw-jekyll/docs/index.html preview. Also, it is including algolia search engine for fly.io, look how search looks in desktop mode.

Screenshot_2022-03-17-10-04-10-307_com android chrome

@vsoch
Copy link
Owner

vsoch commented Mar 17, 2022

ah that's the reason it's not working! Could you please provide that as clear feedback next time, because I was only looking at the explicit opening /closing of the menu and did not look at the search.

I've updated the PR and here is the new preview: https://10-447414597-gh.circle-artifacts.com/0/tw-jekyll/index.html

@heykapil
Copy link
Author

heykapil commented Mar 17, 2022

Thanks @vsoch , Menu is working. Noticed in preview that menu button when clicked to expand items moves a bit left and when clicked to close menu items, it moves bit right Similar thing here https://imkapil.netlify.app/
Just this small tweak and this issue can be closed.

@vsoch
Copy link
Owner

vsoch commented Mar 17, 2022

I'm going to merge the PR since the issue is already present and the PR is an improvement, and if you have a suggested fix I'd be happy to test it out in another one. Thanks!

@vsoch vsoch closed this as completed in #4 Mar 17, 2022
@vsoch vsoch reopened this Mar 17, 2022
@vsoch vsoch changed the title Navbar not working in mobile devices Menu icon in mobile view jumps open->close Mar 17, 2022
@vsoch
Copy link
Owner

vsoch commented Mar 17, 2022

It doesn't bother me that much, so I don't see actively working on it for a fix, but I will leave the issue open if others have suggestions. Thanks again!

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

Successfully merging a pull request may close this issue.

2 participants