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 header contents moving when opening modal #2131

Merged
merged 2 commits into from
May 23, 2020

Conversation

w-4
Copy link
Contributor

@w-4 w-4 commented Apr 21, 2020

Fixes the following minor bug:

Header contents move on desktop when modal is opened.

Steps:

  1. https://discuss.flarum.org
  2. scroll a little down
  3. open a modal (f.e. Login Modal)
    -> Header contents move back and forth when opening and closing the modal. (Chrome)

Reason: Scrollbar is disabled when modal is open. A padding-right is applied to the body to account for that. When header position is fixed, it does not apply the padding.

Changes proposed in this pull request:
Bootstrap modal has a builtin class navbar-fixed-top that fixes that issue.

Reviewers should focus on:
Changed the header css class to position:fixed for @tablet-up.

Confirmed

  • Frontend changes: tested on a local Flarum installation.

Copy link
Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

I have checked this out locally, and can confirm that it fixes the issue.

Copy link
Contributor

@tankerkiller125 tankerkiller125 left a comment

Choose a reason for hiding this comment

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

Looks good

@askvortsov1 askvortsov1 merged commit 4b68645 into flarum:master May 23, 2020
@askvortsov1 askvortsov1 added this to the 0.1.0-beta.14 milestone May 23, 2020
askvortsov1 added a commit that referenced this pull request Oct 7, 2020
@askvortsov1 askvortsov1 mentioned this pull request Oct 7, 2020
askvortsov1 added a commit that referenced this pull request Oct 9, 2020
* Revert "Fix header contents moving when opening modal (#2131)"
* Fix header contents moving when modal opened/closed.

Conditionally apply the navbar-fixed-top class only when needed, so that we can take advantage of it without always having the navbar in position:fixed, as was done in the previous solution. That resulted in a clash with custom headers.

* Show header on refresh of scrolled page

Due to some magic in Mithril 0.1's context:retain flag, some DOM elements were cached across page reloads. Since that has been eliminated, if we refresh the page and we are scrolled down, the "affix" class which makes the header fixed (and as a result, visible) isn't applied until the first scroll. We fix this by running ScrollListener.update() immediately to set initial navbar state.
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.

3 participants