Problem/Motivation

Main menu fills all height of window on smartphones up to 460px width.
See screenshot below.
Current menu screenshot

Proposed solution

Make the main menu collapsible and give possibility to expand/collapse it.
Almost the same solution was implemented and commited to the Responsive Bartik D7 theme. See #1873156: Make menu collapsible on small screen resolutions
Please see screenshots below.
Collapsed:
Collapsed menu
Expanded:
Expanded menu

Demo

You may take a look at implemented solution in action at http://d8.komelin.com

Tests

Implemented solution tested with:

  • Opera, Safari, FireFox, Chrome, IE 9
  • Mobile version of IE 9 (WindowsPhone7)
  • Android native browser (HTC Desire S)
  • responsinator.com

Also, tested some features, such as:

Possible improvements

1) Display menu for clients without JS
2) Add checkbox to the theme settings that will turn on/off this menu feature

Related issues

Additional comments

I set the issue category as Feature request but I think it's just improvement.

What do you think?

Files: 
CommentFileSizeAuthor
#85 bartik-1880488-85.patch5.44 KBrteijeiro
PASSED: [[SimpleTest]]: [MySQL] 58,513 pass(es).
[ View ]
#73 bartik-1880488-73.patch6.5 KBdjroshi
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch bartik-1880488-73.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#65 chart.png1.19 KBkonstantin.komelin
#56 bartik-1880488-56.patch6.5 KBdjroshi
PASSED: [[SimpleTest]]: [MySQL] 52,260 pass(es).
[ View ]
#56 interdiff.txt354 bytesdjroshi
#55 menu-test-ie9-10.jpg24.03 KBmjohnq3
#54 bartik-1880488-54.patch6.52 KBdjroshi
PASSED: [[SimpleTest]]: [MySQL] 52,239 pass(es).
[ View ]
#54 interdiff.txt1.94 KBdjroshi
#52 bartik-1880488-52.patch6.18 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 52,375 pass(es).
[ View ]
#52 interdiff.txt518 bytesjibran
#40 bartik-1880488-40.patch6.16 KBdjroshi
PASSED: [[SimpleTest]]: [MySQL] 50,589 pass(es).
[ View ]
#36 bartik-1880488-36.patch5.12 KBdjroshi
PASSED: [[SimpleTest]]: [MySQL] 50,793 pass(es).
[ View ]
#33 bartik-1880488-33.patch5.22 KBdjroshi
PASSED: [[SimpleTest]]: [MySQL] 50,425 pass(es).
[ View ]
#32 1880488-margins.png1.15 KBkonstantin.komelin
#30 bartik-1880488-30.patch5.1 KBdjroshi
PASSED: [[SimpleTest]]: [MySQL] 50,414 pass(es).
[ View ]
#26 bartik-1880488-26.patch5.1 KBdjroshi
PASSED: [[SimpleTest]]: [MySQL] 49,721 pass(es).
[ View ]
#25 bartik-1880488-25.patch3.82 KBdjroshi
PASSED: [[SimpleTest]]: [MySQL] 50,005 pass(es).
[ View ]
#24 bartik-1880488-24.patch5.96 KBdjroshi
PASSED: [[SimpleTest]]: [MySQL] 49,713 pass(es).
[ View ]
#20 bartik-1880488-20.patch4.65 KBkonstantin.komelin
PASSED: [[SimpleTest]]: [MySQL] 49,299 pass(es).
[ View ]
#12 bartik-1880488-12.patch4.97 KBkonstantin.komelin
PASSED: [[SimpleTest]]: [MySQL] 49,327 pass(es).
[ View ]
#10 bartik-1880488-10.patch4.65 KBkonstantin.komelin
PASSED: [[SimpleTest]]: [MySQL] 50,675 pass(es).
[ View ]
#8 bartik-1880488-8.patch4.64 KBkonstantin.komelin
PASSED: [[SimpleTest]]: [MySQL] 50,800 pass(es).
[ View ]
#1 bartik-1880488-1.patch5.25 KBkonstantin.komelin
PASSED: [[SimpleTest]]: [MySQL] 50,796 pass(es).
[ View ]

Comments

StatusFileSize
new5.25 KB
PASSED: [[SimpleTest]]: [MySQL] 50,796 pass(es).
[ View ]

Patch attached.

Issue tags:+mobile

Tagging with Mobile to get this on the right peoples' radars.

Okay, thanks!

Had problem with RTL language demo but now it's working.

Status:Active» Needs work

See how it's already done in vertical-tabs.js resize event is expensive to use and that's probably not worth it. For now it only checks at page load.

Thanks, nod_. I'll look at vertical tabs approach.

Also try to use <button> that's the right element for the job, not an empty <a> :) some examples at #1719640: Use 'button' element instead of empty links

Status:Needs work» Needs review
StatusFileSize
new4.64 KB
PASSED: [[SimpleTest]]: [MySQL] 50,800 pass(es).
[ View ]

I've done improvements according to #5 and #7.
- It checks client width at page load only (wo resize event handling)
- Replaced <a> with <button>

Attached new patch and updated the demo http://d8.komelin.com.

One issue I see now is menu hides on portrait <-> landscape switching.
Any ideas?

can you use .on('click', function… instead of .click(function… ?

StatusFileSize
new4.65 KB
PASSED: [[SimpleTest]]: [MySQL] 50,675 pass(es).
[ View ]

Replaced .click with .on('click'. Updated demo.

Status:Needs review» Needs work

I am afraid the patch is corrupted by a png image.

Please, review it.

StatusFileSize
new4.97 KB
PASSED: [[SimpleTest]]: [MySQL] 49,327 pass(es).
[ View ]

Re-created patch with command:
git diff --full-index --binary

Checked with command:
git apply -v --check bartik-1880488-12.patch
Results:

Checking patch core/themes/bartik/bartik.info...
Checking patch core/themes/bartik/css/style.css...
Checking patch core/themes/bartik/images/toggle.png...
Checking patch core/themes/bartik/js/collapsible-menu.js...
Checking patch core/themes/bartik/template.php...

Status:Needs work» Needs review

Status:Needs review» Needs work

Sorry but the patch gives me errors when applying:

$ git apply bartik-1880488-12.patch
bartik-1880488-12.patch:71: trailing whitespace.
?PNG
error: patch failed: core/themes/bartik/bartik.info:9
error: core/themes/bartik/bartik.info: patch does not apply
error: patch failed: core/themes/bartik/css/style.css:1520
error: core/themes/bartik/css/style.css: patch does not apply
error: core/themes/bartik/images/toggle.png: already exists in working directory
error: core/themes/bartik/js/collapsible-menu.js: already exists in working directory
error: patch failed: core/themes/bartik/template.php:11
error: core/themes/bartik/template.php: patch does not apply

Could someone test it too?

I am trying to see what's happening.

Status:Needs work» Needs review

The above patch applied cleanly for me except for the whitespace error.

$ git apply -v bartik-1880488-12.patch
bartik-1880488-12.patch:71: trailing whitespace.
89PNG
Checking patch core/themes/bartik/bartik.info...
Checking patch core/themes/bartik/css/style.css...
Checking patch core/themes/bartik/images/toggle.png...
Checking patch core/themes/bartik/js/collapsible-menu.js...
Checking patch core/themes/bartik/template.php...
Applied patch core/themes/bartik/bartik.info cleanly.
Applied patch core/themes/bartik/css/style.css cleanly.
Applied patch core/themes/bartik/images/toggle.png cleanly.
Applied patch core/themes/bartik/js/collapsible-menu.js cleanly.
Applied patch core/themes/bartik/template.php cleanly.
warning: 1 line adds whitespace errors.

I am wondering if we can take a similar approach to the collapsed menu button as Twitter Bootstrap.
Replace the PNG image with an HTML button and some simple CSS.

HTML

<button class="btn btn-navbar" data-target=".nav-collapse" data-toggle="collapse" type="button">
<span class="icon-bar"></span>
<span class="icon-bar"></span>
<span class="icon-bar"></span>
</button>

CSS

.navbar .btn-navbar {
    background-color: #EDEDED;
    background-image: linear-gradient(to bottom, #F2F2F2, #E5E5E5);
    background-repeat: repeat-x;
    border-color: rgba(0, 0, 0, 0.1) rgba(0, 0, 0, 0.1) rgba(0, 0, 0, 0.25);
    box-shadow: 0 1px 0 rgba(255, 255, 255, 0.1) inset, 0 1px 0 rgba(255, 255, 255, 0.075);
    color: #FFFFFF;
    display: none; // removed on breakpoint for small screens
    float: right;
    margin-left: 5px;
    margin-right: 5px;
    padding: 7px 10px;
    text-shadow: 0 -1px 0 rgba(0, 0, 0, 0.25);
}
.navbar .btn-navbar .icon-bar {
    background-color: #F5F5F5;
    border-radius: 1px 1px 1px 1px;
    box-shadow: 0 1px 0 rgba(0, 0, 0, 0.25);
    display: block;
    height: 2px;
    width: 18px;
}
.nav-collapse, .nav-collapse.collapse {
    height: 0;
    overflow: hidden;
}
.navbar .btn-navbar {
    display: block;
}
.btn {
    -moz-border-bottom-colors: none;
    -moz-border-left-colors: none;
    -moz-border-right-colors: none;
    -moz-border-top-colors: none;
    border-image: none;
    border-radius: 4px 4px 4px 4px;
    border-style: solid;
    border-width: 1px;
    cursor: pointer;
    font-size: 14px;
    line-height: 20px;
    margin-bottom: 0;
    text-align: center;
    vertical-align: middle;
}

This would allow the button to be styled if the user wishes to override defaults.

Status:Needs review» Needs work

Hi, I finally applied #12 patch, removing the collapsible-menu.js and toggle.png files, but now the menu completely disappeared.

Should I have to enable or config some modules?

Status:Needs work» Needs review

No, you shouldn't. You should clean up your source base from the patch changes, pull latest changes from 8.x branch, and then apply patch again (if you wish).

Tried again to test but no luck.

My menu disappears after patch apply. I will see what is happening.

Maybe someone could test it too.

StatusFileSize
new4.65 KB
PASSED: [[SimpleTest]]: [MySQL] 49,299 pass(es).
[ View ]

Re-created the patch using git diff --binary.

It does work. I've applied it to the demo site sources http://d8.komelin.com/.

Status:Needs review» Needs work
Issue tags:-mobile

The last submitted patch, bartik-1880488-20.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+mobile

#20: bartik-1880488-20.patch queued for re-testing.

StatusFileSize
new5.96 KB
PASSED: [[SimpleTest]]: [MySQL] 49,713 pass(es).
[ View ]

For your consideration, this is my attempt at a css-only implementation of this design, which I threw together as part of the drupalcon sydney code sprint.

StatusFileSize
new3.82 KB
PASSED: [[SimpleTest]]: [MySQL] 50,005 pass(es).
[ View ]

I have revisited the css and tidied it up a lot, removing superfluous css and a bunch of unneccessary body:not(:target) identifiers - leaving the minimum required to ensure that this functions well in browsers that support :target and degrades nicely in those that don't...

StatusFileSize
new5.1 KB
PASSED: [[SimpleTest]]: [MySQL] 49,721 pass(es).
[ View ]

Looks like toggle.png was omitted from the last patch, otherwise this patch is the same as #25

Status:Needs review» Needs work

I applied the latest patch on my local installation of D8 and also on a sandbox on http://simplytest.me/. In both instances at the screen size that triggers the collapsible menu, it does not toggle. Clicking on the menu button will open the menu but it cannot be closed.

I haven't used http://simplytest.me/ before - I can see that being quite useful, thanks.

I think back in #24 I had it so clicking anywhere that wasn't a menu item would close the menu. In #26 the menu only closes if you click the menu button. I don't really mind either way (as long as it works, of course) so please let me know which behavior you think is best...

I tested #26 against Drupal Core 8.x on http://simplytest.me/ and it seems to be working OK for me. So far tested in Safari, FF, Chrome.

Status:Needs work» Needs review

I reapplied the patch in #26 on a http://simplytest.me/ sandbox and found that clicking on the menu button will open it but the menu can only be closed by clicking on the menu area not the button.

We need a few other people to test this - both approaches - the javascript version and the css only one.

I personally have no particular favorite. Both appear to work, are sound, and are generally in use, although the js is probably more common.

StatusFileSize
new5.1 KB
PASSED: [[SimpleTest]]: [MySQL] 50,414 pass(es).
[ View ]

I have reviewed #20 and I can confirm that it does work - however on a device with javascript disabled the menu disappears completely.

I am hoping this patch will fix the issue with the button not triggering menu close.

Yes, great, the menu works fine now with the above patch.

If we use the javascript approach, the menu would need to shown as is, not hidden, if js were not enabled on the device. I believe the OP was aware of that.

StatusFileSize
new1.15 KB

Tested #30. It works great. Thanks djroshi!

Advantages of CSS approach (#30):
- It works with disabled JS
- It works with changing of device orientation Portrait <-> Landscape
- We don't have to load 'drupal' and 'jquery' libraries for every page

Advantage of JS approach (#20):
- The toggle effect looks a bit sexier (slideToggle('fast'))

I see an improvement for #30. Please take into account margins of toggle button.
Attached screenshot.

So I'd love to see #30 in core. But I think we need some more testing on devices. I tested it on WinPhone7 only.

StatusFileSize
new5.22 KB
PASSED: [[SimpleTest]]: [MySQL] 50,425 pass(es).
[ View ]

Thank you for the feedback :)

I think with the right-hand margin, my browser scroll-bar was getting in the way a bit. I have set it back to 10px as per the original design.

I have also wrapped a couple of hard-coded strings in t() functions.

Used SimplyTest.me to test #33.
The toggle button is not visible. But I might be wrong.

Status:Needs review» Needs work

I tested #33 on SimplyTest.me also. The menu button doesn't appear.

StatusFileSize
new5.12 KB
PASSED: [[SimpleTest]]: [MySQL] 50,793 pass(es).
[ View ]

#33 generates a corrupt toggle.png file - hence the disappearing menu button. No idea how that happened. Hopefully this one works...

I have tested #36 on simplytest.me and the menu button is back. Yay!

Status:Needs work» Needs review

Looks good.

#36 doesn't support RTL languages.

StatusFileSize
new6.16 KB
PASSED: [[SimpleTest]]: [MySQL] 50,589 pass(es).
[ View ]

RTL breaks the standard bartik responsive menu too by the way :)

We actually have a bit of overlap happening here:

http://drupal.org/node/1804446

What I have done is pulled a section of that patch (the main-menu stuff) into #40 (and modified it slightly) so that this issue can address the remaining issues with the bartik responsive main menu.

I hope this is OK?

#40 works well for me.

Patch #40 looks good to me too.

Status:Needs review» Reviewed & tested by the community

Tested, patch applies cleanly with one insignificant whitespace error. Patch works as designed and solves the problem.

Marking RTBC.

I tested it works great.
But I think we need e.preventDefault(); or javascript:void(0);

+++ b/core/themes/bartik/template.phpundefined
@@ -75,6 +75,15 @@ function bartik_process_page(&$variables) {
+  // Add a hide menu item to the main menu
+  if ($variables['main_menu']) {
+    $variables['main_menu']['menu-hide'] = array(
+      'title' => t('Hide menu'),
+      'href' => '',
+      'fragment' => ' ',
+      'external' => TRUE,
+    );

But I think we need e.preventDefault(); or javascript:void(0);

Why?

Because it will stop browser from bubbling up when menu icon is clicked.

There is no javascript in #40 - these click events need to be handled by the browser.

Status:Reviewed & tested by the community» Needs review

Sounds like this needs more discussion?

Patch #40 uses CSS to toggle the menu. It does so by using an anchor href and the CSS3 :target selector. The menu opens by navigating the browser to "#main-menu-links" and closes by navigating to "#".

Of course "#" is commonly misappropriated as a href for elements that are solely used as javascript triggers. In these cases javascript:void(0) and e.preventDefault() is used to stop events being handled by the browser.

I would appreciate an explanation from jibran as to how exactly his comments are at all relevant to the patch provided.

Sorry for not being clear. Let me explain myself.

  • After applying the patch.
    Only local images are allowed.
  • After clicking the menu icon
    Only local images are allowed.
    see site logo is not visible only menu items are showing at the top.
  • If we'll usee.preventDefault(); or javascript:void(0); it would look like this
    Only local images are allowed.
    browser will scroll down and site logo is still visible.

So all I am saying is that page have to remain at its original place and it should slide down to accommodate menu items.
And IMHO menu items should appear under menu icon not the other way around. So adding the usability tag for more opinions.

Please read #32 for a brief comparison of the CSS and JS approaches, and review #20 if you are interested in a javascript implementation.

The CSS approach is obviously limited by cross-browser issues. The menu items all have fluid height which is difficult to animate in CSS and the use of the :target selector results in a page jump to the named div. That said it's very, very lightweight, doesn't require javascript to be enabled, and is designed to degrade nicely if browser doesn't support the CSS :target selector.

Also, I have to say. You click the menu button - it jumps to the menu. OMG.

I think I have already explained why e.preventDefault() and javascript:void(0) are not applicable solutions. If you are not convinced then I guess you will have to try it, and see for yourself that it completely breaks the functionality. If you are going to tell the browser to not respond to click events, you better have a script or something that does - or else guess what? Nothing happens.

As for menu items above/below, I'm not really bothered either way.

To sum up, #40 has been tested as working by four people besides myself. It has two known "issues", the page jump due to named anchors, and the lack of a slide animation. Both these issues directly reflect the trade-off between the CSS and JS approaches.

IMO Bartik should be lightweight and functional. Loading jquery on every page just to enable slide animation for a toggle menu that only appears on mobile devices just doesn't seem right to me...

StatusFileSize
new518 bytes
new6.18 KB
PASSED: [[SimpleTest]]: [MySQL] 52,375 pass(es).
[ View ]

Ok let's try

#52 Collapsing doesn't work at least in FireFox and Chrome. Q.E.D.

StatusFileSize
new1.94 KB
new6.52 KB
PASSED: [[SimpleTest]]: [MySQL] 52,239 pass(es).
[ View ]

This should maybe fix the page jump issue. However cross-browser compatibility now depends on both :target and adjacent sibling ~ CSS selectors being supported. I would be interested to see how well supported this is on mobile platforms. Tested in Firefox, Chrome, Safari, and Opera.

StatusFileSize
new24.03 KB

Tests fine using a sandbox on simplytest.me on Chrome 25, Firefox 19, and Opera 12.5. It also works correctly with IE9 and IE10 but there's a slight visual anomaly, (stray text appears below the word Menu when its opened) as shown in the attached screenshot.

StatusFileSize
new354 bytes
new6.5 KB
PASSED: [[SimpleTest]]: [MySQL] 52,260 pass(es).
[ View ]

Hmm, I was using a slightly newer version of the -999em hack to hide that text. Maybe the original will work better? Sorry I can't test in IE at the moment :(

Tested #54.
Desktop:

  • FireFox, Chrome, Opera, Safari - fine
  • IE9 - fine, but with text-indent effect described in #55

Mobile:

  • IE9 on WindowsPhone7 - the same effect #55 and short delay between click (touch) and reaction. I guess the delay depends on concrete device.

It is working fine in IE9, IE10, Chrome 25, Firefox 19. Thanks for the changes.

#56 works well in the latest desktop browsers (Windows) and IE9 on WinPhone7.

Issue tags:+Usability

Adding tag.

#56 works correctly in IE9 and IE10.

So nod_ pointed me here. I’ve done a redesign of the way our navigation tabs (e.g. at the top of the overlay) wok on small screens. Very similar to the standard approach to responsive navs, and to the interaction pattern applied here. However, my solution uses JS to detect available space and switch layout accordingly. Uses CSS animations for performance on small screens.

A demo is available here: http://drupalcode.org/sandbox/ry5n/1932040.git/blob_plain/ec02de3c6a05ef...

I wonder if we could decide on a best implementation for core? A basic solution that if needed, one or both (navs/tabs) can extend?

@ry5n, we have at least two working solutions in this issue:
1) #1 jQuery.resize is used to check size. Works everywhere, supports landscape -> portrait switching.
2) Excelent solution without Javascript dependency #56. Works. Tested.

You suggested 3d. Let's compare them.

StatusFileSize
new1.19 KB

Shortened url of @ry5n demo: http://goo.gl/ZLa5f
Should be better now:
chart.png

#63 doesn't look very responsive on Windows Phone 7. I guess due to lack of meta:viewport tag

Btw, now it's possible to generate QR code for simplytest.me sandbox url by clicking special icon on the timer panel.

I’ve added a viewport meta tag and fixed some JS bugs in my implementation. Demo of the new commit is at http://bit.ly/YLcw6U

To compare all three approaches, I’m only looking at code and the post in this issue; it looks like both #20 (the most recent JS-based patch) and #56 need re-rolls.

#56. CSS-only approach: no JS needed (nice). Some of the CSS selectors look more complex than they need to be, but that doesn’t detract from the approach. Downsides mentioned in previous comments are the lack of an open/close animation and the jump caused by scrolling the page to the named anchor. I would have to try a demo to see if the experience was acceptable. Sometimes named-anchor jumps are disorienting.

#20. Basic JS approach. Obviously requires JS. Has a nicer animation for open/close (although animating in JS on mobile can perform poorly, i.e. low FPS).

An additional downside of both approaches is they depend on specific media queries to toggle layout, so both would need manual tweaking if and when the menu items change.

#67. This one :)
Advantages: falls back to a stacked nav when no js. Touch targets larger on touch screens. Layout switch happens automatically based on the actual rendered size of the element (no media queries to manage). Has a nice slide animation; uses hardware-accelerated CSS transitions for smooth performance on CPU-constrained devices (which is where it would be seen).

Disadvantages: complex. Depends on jQuery, Modernizr (touch, csstransitions). Uses several custom jQuery plugins (they are modular though, none is particularly complicated). Does use a window.resize handler. In reality though, if we assume folks won’t resize their window, the performance cost of having the resize handler is still zero :) And if the window does resize, it doesn’t break.

For me, it comes down to a couple of things. Drupal’s admin UI (Seven) needs to provide a really good experience. That means speed, but also avoiding aesthetic and usability issues (potentially) with the CSS approach. Again, I’d need to demo it to see. If we want to go with a single approach for responsive navs in Core, it should be one that can support all core themes.

So: should we build a common responsive nav implementation? If so, my tentative vote is for JS with auto-layout.

> if we assume folks won’t resize their window, the performance cost of having the resize handler is still zero :)
This is actual for #1 as well. I mentioned it instead of #20 because #20 doesn't use resize and therefore doesn't support landscape <-> portrait switching.

If you wanna see how the similar approach (#1) works take a look at Responsive Bartik theme demo http://drupal.org/project/responsive_bartik

I personally prefer #56. The named-anchor jump is not disorienting users because the anchors are close to each other.

Let's see what other guys think.

@konstantin.komelin

This is actual for #1 as well.

Yes absolutely. My argument about resize handlers applies generally.

I looked at responsive_bartik, but it doesn’t seem to include the changes proposed here. I also tried http://d8.komelin.com/ but I don’t see the expected behavior. Do I need to log in?

I’d like to try both of these out, either a demo or reroll. Can someone oblige?

http://d8.komelin.com/ is not up to date.

Reroll is probably ideal variant because we will be able to use simplytest.me for applying patches and seeing results in action.

Written message to @djroshi about #56

StatusFileSize
new6.5 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch bartik-1880488-73.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Reroll of #56

Status:Needs review» Needs work

The last submitted patch, bartik-1880488-73.patch, failed testing.

Status:Needs work» Needs review

Thanks djroshi! #73 looks good.

#73: bartik-1880488-73.patch queued for re-testing.

Thanks for the reroll! I still think we need JS rather than hard-coded breakpoints to switch the menu layout, for flexibility reasons. But now that I’ve tested this, I feel better about the CSS-only solution for the toggle action. It works well; the only disconcerting thing is that the menu appears above the trigger, rather than below. I feel like the place where my attention was focused shouldn’t jump away, and I should be able to tap the same spot to reverse the action.

I also realized that if we were to use the CSS-only approach as a general solution for core it would need to work inside the Overlay (because of the nav tabs). But the Overlay uses special URLs like /#overlay=admin/content, which is probably an issue since there’s already a fragment in there.

Overlay doesn't work well (at all?) on touch so we just get rid of it when that's the case (I think some code needs to be updated for that to work well but that's the idea).

I wouldn't worry too much about responsive things inside the overlay. There shouldn't be any.

That’s true. In an ideal world it’s not only about touch or screen size, since we could still have a set of tabs that don’t fit across the overlay (especially with the toolbar in vertical mode), but I guess if it works enough of the time, it’s OK.

Can we get get a consensus on whether the menu should appear above (as per konstantins original design) or below the menu button?

Much time has passed. I'm giving you guys opportunity to decide.

#73: bartik-1880488-73.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Needs usability review, +Usability, +mobile

The last submitted patch, bartik-1880488-73.patch, failed testing.

Assigned:Unassigned» rteijeiro

Re-rolling...

Status:Needs work» Needs review
StatusFileSize
new5.44 KB
PASSED: [[SimpleTest]]: [MySQL] 58,513 pass(es).
[ View ]

Patch re-rolled.

+++ b/core/themes/bartik/templates/page.tpl.php
@@ -144,6 +144,8 @@
+        <div id="nav"></div>
+        <div id="no-nav"></div>

Have talked with LewisNyman and maybe we should refactor this empty divs.

Sure. What do you suggest?

Status:Needs review» Needs work

Having read through the issue now, I think the CSS only approach is a decent improvement, the JS based tabs solution is still being implemented in Seven and maybe at a later date we could look at solving it in the same way. I don't think it's a major problem here though.

Let's fix the final UX issue and call it done.

Can we get get a consensus on whether the menu should appear above (as per konstantins original design) or below the menu button?

Above.

OK I will provide a patch with the menu below the button so we can do a proper comparison...

Assigned:rteijeiro» Unassigned