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: z-indexes of top menu, dropdown and subnavs #928

Merged
merged 4 commits into from
Mar 15, 2019

Conversation

MMFernando
Copy link
Contributor

@MMFernando MMFernando commented Mar 14, 2019

Changed z-indexes so that the elements are properly placed and the dropdown menu gets closed when the user scrolls so it doesn't become unreachable on Rapid Router's page.


This change is Reviewable

Copy link
Member

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mariaFernando)


portal/templates/portal/base.html, line 363 at r1 (raw file):

    $(window).on('scroll', function() {
        var scroll = $(window).scrollTop();
        if ((scroll > 0) && ($('.dropdown').hasClass('open'))) {

I think you can have the if without the inner brackets around scroll > 0 and the other condition


portal/templates/portal/base.html, line 365 at r1 (raw file):

        if ((scroll > 0) && ($('.dropdown').hasClass('open'))) {
            $('.dropdown').removeClass('open');
            $('.button--dropdown').attr("aria-expanded","false");

Space between the comma and "false"

@MMFernando
Copy link
Contributor Author


portal/templates/portal/base.html, line 365 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Space between the comma and "false"

There is actually no space there x)

Copy link
Member

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mariaFernando)


portal/templates/portal/base.html, line 365 at r1 (raw file):

Previously, mariaFernando (Maria Mafalda Fernando) wrote…

There is actually no space there x)

I know, I'm asking you to put one in xD

Copy link
Member

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @mariaFernando)

@MMFernando
Copy link
Contributor Author


portal/templates/portal/base.html, line 365 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

I know, I'm asking you to put one in xD

ohhOOOHHH

Copy link
Member

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r3.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor Author

@MMFernando MMFernando left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


portal/templates/portal/base.html, line 365 at r1 (raw file):

Previously, mariaFernando (Maria Mafalda Fernando) wrote…

ohhOOOHHH

Done.

@MMFernando MMFernando merged commit 4c28b53 into master Mar 15, 2019
@MMFernando MMFernando deleted the menu_hidden_by_subnav branch March 15, 2019 10:39
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