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

Fix hamburger menu on mobile layout: check array index #59

Merged
merged 1 commit into from
Oct 5, 2020

Conversation

pythonquick
Copy link
Contributor

When tapping the hamburger menu icon on mobile layout, nothing happens. It turns out there's a syntax error with:

altArr[1]?.split(' ')

This fixes the ? and also checks for array index

@onweru
Copy link
Collaborator

onweru commented Oct 2, 2020

@pythonquick,

The commit looks ok. However, I couldn't reproduce the bug you described. That's even without including the change you've proposed in this PR of course.

Steps to reproduce? Os, browser ... maybe even a url?

@pythonquick
Copy link
Contributor Author

@onweru
sure, i can reproduce it by loading the Clarity demo page: https://themes.gohugo.io/theme/hugo-clarity/ on my iPhone 8 device. Tapping on the hamburger menu in the top right of the page does not expand the panel on the left. It happens both on Safari and Chrome apps on the iPhone. My iPhone is on iOS 13.3.1

The hamburger menu works fine on an my Android tablet and on my iPad.
When i load the same page on a browser on a laptop (not a mobile device), the hamburger menu works fine.
Looks like it's specific to iPhone.

@onweru
Copy link
Collaborator

onweru commented Oct 3, 2020

Looks like it's specific to iPhone.

Interesting, the demo works flawlessly on my iPhone 11, iOS 14, both on safari & Firefox.

Will wait out this a while longer before merging. Thanks for bringing this up.

@onweru
Copy link
Collaborator

onweru commented Oct 5, 2020

@pythonquick, thanks for contributing. Merging because, in retrospect, your change makes the code more readable.

@onweru onweru merged commit cfccd25 into chipzoller:master Oct 5, 2020
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 this pull request may close these issues.

None yet

2 participants