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

[UX] Drop-down menus: 1st item in long menus is hidden under the admin bar. #2161

Open
klonos opened this issue Sep 12, 2016 · 10 comments · May be fixed by backdrop/backdrop#1684
Open

[UX] Drop-down menus: 1st item in long menus is hidden under the admin bar. #2161

klonos opened this issue Sep 12, 2016 · 10 comments · May be fixed by backdrop/backdrop#1684

Comments

@klonos
Copy link
Member

klonos commented Sep 12, 2016

Follow-up to #2107 ...with the SmartMenus in core, long drop-down menus are handled pretty well:

backdrop-smartmenus-long_menus-first_item_hidden_behind_admin_bar

...the only issue as you see is that if the the first item is scrolled over the top of the viewport, than it get hidden behind the admin bar.

Not so serious, as it will only affect the site admin (other users don't normally have permission to view the admin bar at all). Can also be easily solved with a z-index increase I guess so that the menu is over the admin bar 😉

As with #2160, if we can solve this before 1.5.0, good. If not, then 1.5.1 as a usability bug 😉

PS: I can't wait to test SmartMenus on admin bar. It will most likely solve #520 / #1001


PR by @klonos backdrop/backdrop#1684

@quicksketch
Copy link
Member

Yeah this is just a z-index issue. The problem we have is that the entire admin menu is given a very high z-index, because when it's opened we want it to display over the top of other things (including the SmartMenu in the body). We'll need specific targeting on the sub-menus of SmartMenu to be over Admin Bar.

@quicksketch quicksketch changed the title Drop-down menus: 1st item in long menus is hidden under the admin bar. [UX] Drop-down menus: 1st item in long menus is hidden under the admin bar. Sep 15, 2016
@quicksketch quicksketch modified the milestones: 1.5.0, 1.5.1 Sep 16, 2016
@quicksketch quicksketch modified the milestones: 1.5.1, 1.5.2 Oct 17, 2016
@quicksketch quicksketch modified the milestones: 1.5.2, 1.5.3 Nov 16, 2016
@klonos
Copy link
Member Author

klonos commented Dec 20, 2016

...turns out the dropdown menus receive an inline z-index of 1001 upon expanding (via JS) while admin bar has 999, so this issue shouldn't be happening at all. Right?

@klonos
Copy link
Member Author

klonos commented Dec 20, 2016

...The z-index is applied upon menu expansion, but removed upon starting the scroll. Huh.

@klonos
Copy link
Member Author

klonos commented Dec 20, 2016

...filed an issue upstream.

@vadikom
Copy link

vadikom commented Dec 21, 2016

This should be quite simple to fix but please provide me with some kind of test URL (or explain how I could run a test case locally) so that I could test and verify the best solution. In the worst case it might involve a couple of JS lines since, as far as I understand, you would like your admin bar sub menus to appear over the SmartMenus bar but still the SmartMenus sub menus should appear over the admin bar.

@klonos
Copy link
Member Author

klonos commented Dec 21, 2016

Thanx for taking the time to respond @vadikom and for offering to help with this.

you would like your admin bar sub menus to appear over the SmartMenus bar but still the SmartMenus sub menus should appear over the admin bar.

Precisely! I just filed a PR with my previous attempt to solve the issue (using only CSS). Our admin bar has a z-index of 999, so I am trying to set all menu items on the SmartMenus-rendered menu after the 2nd level to a z-index of 1000.

Each PR filed against Backdrop core creates a sandbox installation that we can test. Here's the link to the one I created and the credentials for it:

Website: http://1684.backdrop.backdrop.qa.backdropcms.org
Username: admin
Password: XUQSwqbX

...I have already created a "very long menu" to help you out with your testing.

@klonos
Copy link
Member Author

klonos commented Dec 21, 2016

The <ul> in the sandbox seems to be getting a z-index of 999 (same as the admin bar). Huh?!

@vadikom
Copy link

vadikom commented Dec 21, 2016

OK, you could achieve it via CSS too - for example, you could add the following:

.js .menu-dropdown {
  z-index: auto;
}

.js .menu-dropdown ul {
  z-index: 1000;
}

in your "menu-dropdown.css".

In case you need to set specific z-index for .menu-dropdown other than auto then a JS solution would be needed that would temporary set the z-index for .menu-dropdown to a higher value than the one used to your admin bar when a sub menu has to be displayed.

BTW, I saw you have edited the default z-index in "sm-core-css.css" - please note that in general it is not recommended editing "sm-core-css.css" at all since in some case that file might be changed on new version release and it would make it harder for you to upgrade if you need to in the future.

@serundeputy serundeputy added this to the 1.6.1 milestone Jan 14, 2017
@serundeputy serundeputy removed this from the 1.5.3 milestone Jan 14, 2017
@quicksketch quicksketch modified the milestones: 1.6.1, 1.6.2 Feb 13, 2017
@quicksketch quicksketch modified the milestones: 1.6.2, 1.6.3, 1.6.4 Mar 15, 2017
@jenlampton
Copy link
Member

No PR, removing milestone.

@stpaultim
Copy link
Member

Just noticed that there does appear to be a PR for this issue, but it's a very old PR and I can see problems with it right off the bat. Someone should probably verify this issue on a more recent version of Backdrop and then give the PR additional attention.

Home___PR_1684_backdrop_backdrop_testing_site

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants