Fly-out dropdown menu in new sidebar
What does this MR do?
Adds a fly-out dropdown menu to the new sidebar making it easier & quicker to access sub-items.
Screenshots (if relevant)
What are the relevant issue numbers?
Closes #34026 (closed)
Merge request reports
Activity
added 1 commit
- 1a2d180e - changed text color for reseting & hover on links
assigned to @cperessini
@iamphill this is working great, and you implemented it so fast!
I noticed the dropdowns close automatically if you hover outside. Would it be possible to introduce a small delay before the dropdown disappears so you have a chance to get your pointer back inside? I think somewhere between 200 and 500 ms would work best.
Do you think it will be possible to do something like Amazon's dropdown for the navigation tunnels?
When the dropdown reaches the bottom of the page the status bar blocks the last row.
I think we can solve this by increasing the
margin-bottomon the sidebar content to 32px and making the fly-outs and the sidebar elements align at the bottom when there is no space to expand them towards the bottom.
added 274 commits
-
1a2d180e...d4c4dec8 - 272 commits from branch
master - 4d2be5bb - Merge branch 'master' into sidebar-fly-out-sub-nav
- 72538b5f - improvements to positioning of the dropdown
-
1a2d180e...d4c4dec8 - 272 commits from branch
@cperessini Made some changes - could you please have another look.
I tried to recreate the navigation 'tunnels' with CSS instead of having a load of JavaScript in it. Let me know if it works ok, if not i'll have a look at the JS solution.
-
@iamphill that's a huge improvement!
I think the CSS implementation is working great, too. Why did you think we may have needed to use JS?
Only thing I noticed is that the delay before closing seems to behave differently depending on which side your cursor leaves the dropdown
added 1 commit
- 80b55398 - added extra padding around fly-out menu to delay hiding
assigned to @filipa
-
@iamphill we are going to consolidate all the dropdowns in the app with a white background and a grey hover style (see #35424 (closed)). Do you think it'd be better to make those changes in this MR or do you prefer to do that in a different one?
@cperessini Are the colors etc. finalised in that issue? If they are, I might as well do it here
-
@iamphill they are the final colors
Element Background Text Sidebar hover #fff$gl-text-colorFly-out resting #fff$gl-text-colorFly-out hover $gray-darker$gl-text-colorHere's a design with the white fly-out:
It looks like a normal dropdown, with the typical
#e5e5e5border. Winnie did some work for this style here !13106 (merged)Please also note this comment about the width of the hover highlight https://gitlab.com/gitlab-org/gitlab-ce/issues/35424#note_35958796
@iamphill code lgtm
I see that this is WIP and that @cperessini left some comments :) Please assign back once it is ready to test locally!
assigned to @iamphill
mentioned in issue #34026 (closed)
mentioned in issue #28921 (moved)
@cperessini this is what it looks like when hovering over.
What colors did you think for the badge? I kind of like it in purple.
assigned to @cperessini
-
@iamphill purple looks nice, but the change from grey to purple looks a bit strange to me. I would keep the badge with the same grey color.
Could you change the text color on the hovered sidebar element to our primary text color? It looks like it's using the secondary one. For the fly-out I think we should use the primary color independently of hover state so it's consistent with other dropdowns.
Is it possible to give the sidebar a higher Z-index than the fly-out? If you zoom in you can see the border of the fly-out on top of the sidebar and I think its drop shadow may be showing on the sidebar a little bit too.
<img src="/uploads/7821136d42eac3161e1a25118242a638/Screenshot_2017-07-27_19.43.08.png" width=253px">
There's a 4px margin to the left of the hover color. I guess it's there because of the active element highlight, but would it be possible to just have a 1px margin on either side like in the fly-out?
And one last thing, I think there used to be some padding at the bottom of the fly-out when it was purple, right? I'm thinking we should have that padding at the top and the bottom because we have it in other dropdowns, but I think you may have removed it because it created strange alignment with the sidebar. Is that right?
added 1 commit
- 948deb97 - updated hover text color of main links in sidebar
-
@iamphill awesome!! Also checked out the branch and I think the alignment with top and bottom padding is good
There seems to be a little extra padding on the right side of the fly-out when you hover
Is the delay not happening anymore? It seems the fly-out is closing automatically now.
@cperessini should still be there - i didn't touch any of that code
Fixed that alignment bit though.
-
@iamphill could you check if it's working like you expect? It seems to work some times and not others:
Working Not working 
assigned to @iamphill
@cperessini i think the delay was happening previously because you weren't moving the mouse as far away so the CSS was still working. I've added some JS, can you let me know if its ok or if I need to increase the timeout?
-
@iamphill you're right, that's exactly it. I think something similar is still happening though.
Check out this example. With your new code I'm able to get the mouse back in the dropdown but it still closes anyways. It's like the dropdown is not listening to the order not to close.
-
@iamphill By the way, you made it so the 'hoverable' area of the fly-out is larger than the fly-out itself, right? I think we can rely exclusively on that and remove the delay logic. Maybe make the 'padding' a tiny bit bigger than it is now and it'll be perfect.
@cperessini Yeah I think the area is about 20px bigger. So I could maybe make it 30px?
-
@iamphill let's go with 30px yeah. I just saw where that is in the code, so I can play around with it and submit a MR myself if I think it needs changing.
Thanks a lot!
assigned to @filipa
-
@iamphill code LGTM but found this:
assigned to @iamphill
@filipa ah nice find! Totally forgot about mobile!
& the EE version https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2576
assigned to @filipa
added 721 commits
-
25d6a6c4...88958e5a - 720 commits from branch
master - e4c20cd3 - Merge branch 'master' into sidebar-fly-out-sub-nav
-
25d6a6c4...88958e5a - 720 commits from branch
-
Thanks @iamphill LGTM!
enabled an automatic merge when the pipeline for e4c20cd3 succeeds
mentioned in commit beaa0723
