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

Use event listener to restore header on expand window #1933

Merged
merged 3 commits into from
May 23, 2022

Conversation

elroygohjy
Copy link
Contributor

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • Others, please explain:

Overview of changes:
Fixes #1922

Anything you'd like to highlight / discuss:
I added event listener to listen resize event, as apparently window's innerwidth remains unchanged on clicking the expand window button.

Testing instructions:
Navbar reappears on expanding window/pressing expanding window button.

Proposed commit message: (wrap lines at 72 characters)
Fix navbar disappearing on expanding window

Copy link
Contributor

@jonahtanjz jonahtanjz left a comment

Choose a reason for hiding this comment

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

Thanks for contributing to MarkBind @elroygohjy 👍

One suggestion, other than that it looks good.

Comment on lines 55 to 60
window.addEventListener('resize', () => {
if (window.innerWidth > 767 && headerSelector.hasClass('hide-header')) {
headerSelector.removeClass('hide-header');
headerSelector.css('overflow', '');
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe can shift this out of the toggleHeaderOnScroll method to line 110 so that this event listener is only added once?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I refactored it.

Copy link
Contributor

@jonahtanjz jonahtanjz left a comment

Choose a reason for hiding this comment

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

Thanks again for fixing this @elroygohjy :)

LGTM 👍

@jonahtanjz jonahtanjz added this to the 4.0 milestone May 23, 2022
@jonahtanjz jonahtanjz merged commit 3854134 into MarkBind:master May 23, 2022
@jonahtanjz
Copy link
Contributor

@all-contributors please add @elroygohjy for code

@allcontributors
Copy link
Contributor

@jonahtanjz

I've put up a pull request to add @elroygohjy! 🎉

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.

Top navigation bar disappears if resize from smaller screen to maximum
2 participants