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

Include Navbar functionality in Header on thinner screens #249

Merged
merged 6 commits into from
Jun 30, 2021

Conversation

renonick87
Copy link
Contributor

Fixes #248

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

This PR was tested in Chrome, Firefox, Safari, and Safari on mobile. NOTE: Having basic authentication enabled adjusts the formatting of the header since a separate new button will be visible, so this PR was tested both with and without basic authentication enabled.

Summary

This PR updates the functionality of the hamburger button in the Header that appears on smaller screens, usually when the sidebar is not visible. It will now open a dropdown nav similar to the sidebar nav. If the screen is resized to be large enough (the threshold is about 768px wide) both the button and the nav will disappear and the side bar will reappear. The sidebar and dropdown do not appear at the same time, so the CSS for both and other additional pages needed to be updated so the formatting wouldn't break on certain screen sizes.

CLA

<a class="nav-link" href="#">Applications/</a>
</li>
-->
<div class="navbar-collapse d-lg-none d-xl-block" data-toggle="collapse" v-if="!isHidden && (innerWidth < 768)" id="navbarsExampleDefault">
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the innerWidth if condition needed here? I removed only that expression in the if statement and seemed to get the same behavior on my browsers. Does something undesirable happen when it's removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The innerWidth check is there to handle a specific case where basic authentication is enabled, and so the user login icon displays in the header alongside the burger button.
Start by having a small browser window, open the navbar from the burger button but don't close the navbar, then resize the window to be larger so that the navbar disappears. You'll see that even though the navbar is not visible, the user icon is forced to the left side of the screen, sometimes overlapping with the SDL logo. This happens because the CSS is saying that the div should not be visible in a larger screen, but the v-if condition is stating that the div should still be present, so you get an invisible div messing with the formatting.
Screen Shot 2021-06-24 at 10 15 44 AM

@renonick87
Copy link
Contributor Author

Fixed an issue where if basic authentication is enabled, then upon loading the login page the UI would attempt to update the pending application count but would fail because the request is unauthorized since the user has not logged in yet. This failed request would redirect the user to the applications page, which would redirect to the login page, and the page would be stuck in an infinite loop.
To fix this, these specific failed requests will no longer redirect to another page, and the pending application count will be updated after the user has logged in.
This PR is again ready for review.

@renonick87 renonick87 merged commit 27bcfd1 into develop Jun 30, 2021
@crokita crokita deleted the feature/navbar-toggle branch July 26, 2021 19:51
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.

Navbar-toggler has no functionality
2 participants