Skip to content

Fix mobile toggle menu functionality #32

Open
amiii123malviya wants to merge 1 commit intohelfi92:masterfrom
amiii123malviya:fix/mobile-toggle-menu
Open

Fix mobile toggle menu functionality #32
amiii123malviya wants to merge 1 commit intohelfi92:masterfrom
amiii123malviya:fix/mobile-toggle-menu

Conversation

@amiii123malviya
Copy link
Copy Markdown

Hello @helfi92 ,

I hope you are doing well,

I have done the changes or solved the issue, I hope it will be valuable.

Best Regards

Comment thread index.html
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation changed in this file. Let's revert them.

Comment thread controller.js
const toggle = e => e.classList.toggle('is-active');
const toggleNav = ({ target }) => Array.from(navMenu.classList).includes('is-active') ? toggle(navMenu) : null;

hamburger.addEventListener('click', () => toggle(navMenu, 'is-active'));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I replicate the issue on my end? When I go to https://hassanali.me/studorlio/ using the mobile view, the hamburger menu works for me. I may be missing something.

screencast.2024-07-18.20-00-22.mp4

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @helfi92 ,

I hope you are doing well.

I've reverted the changes to controller.js and restored it to its previous state.
And
It looks like the hamburger menu is working fine for me on mobile view, as well and I'm not able to replicate the issue. Since this issue was reported by someone else, it might be specific to certain devices or conditions.

Best Regards

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asked the author of the issue for more information - #30 (comment)

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.

2 participants