Kill the overlay for widths below 640 pixels

Sounds like this could possibly be backported.

Also a bug.

Nice to see reports coming in from the sprint. Plz use generic 'usability' tag as well :)

After thinking about this for a while, I think we need a [meta] issue for mobile. Rather than us creating 10+ majors straight of the bat. We now have "some" idea of what issues are for mobile, lets get a more well put together analysis and than prioritize.

I have to admit that I was initially unexcited about converting all of Drupal's admin interfaces to be mobile friendly, but it really is the first experience someone has with Drupal in a mobile context. First impressions are critical.

FOR EXAMPLE, trying to interact with Drupal.org's issue queue on a phone (as I am doing) is pretty off-putting. :-p

Should we simply replicate the existing desktop features in mobile? It seems to me like the overlay is designed to do the following:

• Provide a modal dialog.
• Offer some permanence so users don't think they've gone anywhere

Mobile devices have very different paradigms for accomplishing both of these goals. Often, we see the screen "wipe" left or right to signify that you're going somewhere that you might come back from. And to keep a user's place, a back button is placed at the top of the UI on the side opposite the wipe to signify that you'll head "backwards" by following the link.

There's also JS performance to consider. The overlay was designed for desktop. Mobile phones don't offer the same performance, and an enormous component of user experience is the snappiness of the UI.

I don't yet know what the answer is, but at the moment my hunch is that an overlay will only work on large screens. We shouldn't abolish the modal functionality, but I'd like to consider the best pattern for mobile use.

During Overlay development #787896: Add a link so that administrators can return to their most recently visited non-admin page was opened. It might be worth considering somehow for this issue.

Re: rupl #7

Thats's a good point. It would be really cool if clicking on an overlay link brought up the admin page via a page slide and then clicking the back button did a slide back to where you were. That would be more useful on tablets/smartphones than the current functionality which I agree is really only useful on the Desktop.

I'm going to agree with #7. We really don't need or want overlay on small screens. It doesn't work.

I like #7. But maybe we can tackle this in 2 parts?

1. Disable the desktop overlay for small screens and touch devices. More specifically: prevent the JS for the overlay from loading under those conditions, while ensuring admin links still work.
2. Build an interface for a modal-based mobile admin that has the ability to allow users to return to their original, non-admin spot on the site.

#1 is actually very high on the to-do list for the Mobile Initiative. Bumping to Major.

Those wondering why "touch devices" is included in this (as I was) should take a look at the iPad issues docemented in the sprint results: https://docs.google.com/spreadsheet/ccc?key=0AgPrMtcLgOljdDJUZkc1bE9rWXJ...

Are we certain 960 is the cut-off point for minimum width?

I'm glad everyone is on the same page about the overlay. I've reworked the title to reflect consensus.

@lewisnyman - If you get stuck, don't worry about the technical aspects of creating this stuff. Many of us would gladly take that work on and will be happy to assist you. In fact, here's a live demo I created for simple CSS3 flip cards using 3D transforms long ago in preparation for this day. Two of the examples have forms on the flip side, and one even says "Configure block" ... where have we all heard that before?? (it only appears on :hover which I am aware does not exist on today's touch screens)

Bonus: I just spiffed it up to work with FF10 in addition to the usual webkit suspects (Safari, Chrome, mobile flavors).

edit: I updated the broken link in July 2016

Actually, that is very generic - this issue is specifically about solving an issue regarding the overlay. Making modal dialogs useful on mobile would require a separate issue.

Bojhan reminded me how urgent this is to work on other tasks. Let's try to get a patch for this asap. Adding to the "mobile sprint" issues.

It seems to me #8, along with a mechanism to conditionally load the overlay using a mechanism to reliably test for touch (see #12), would be a quick fix to this while a longer-term, elegant UX solution is worked out.

The general approach of #18 seems good to me. However, it might be worth it to create a function that tests multiple properties and utilize the function in the two conditionals. Less code duping too, easier to maintain.

As an example, many current versions of Chrome will NOT report TouchEvent as undefined, so they will never get the overlay using patch #18.

This might help with testing. Shows what your browser reports, along with loads of Browserscope results: http://modernizr.github.com/Modernizr/touch.html

Ok, this needs a generic solution. Today it's overlay, tomorrow it's vertical tabs ; there needs to be a function dealing with this stuff or I'll have to clean up later anyway.

indeed :þ

Summary of discussion at the mobile sprint.
The BEST way to solve this problem is to have a generic javascript loader in core.
Overlay javascript could then be loaded, dependent on passing certain tests that this generic solution provides.
There are a number of issues relating to a script loader
#1033392: Script loader support in core (LABjs etc.) for example
or #1423500: Use RequireJS to load all JS

This is the best interim solution.

This, and perhaps all Drupal javascript, would need to be rewritten once a script loader landed in core.

+++ b/core/modules/overlay/overlay-parent.jsundefined
@@ -1,6 +1,12 @@
+if((document.documentElement.clientWidth > 640) && (typeof TouchEvent == "undefined")) {

Missing space after if. Why not reverse the logic and return, where applicable? That could be a function.

In the mean time, please *do* get this interim solution in to allow for more mobile work to continue. Consider how 'perfect' this has to be in light of the generic loader solution.

Here you go, that works.

Might want a Drupal.checkMobile function in the end but I don't know if it's possible to reliably check for that.

Note that this patch breaks overlays URL like drupal-8/#overlay=%3Fq%3Dadmin%252Fmodules I personally don't care but good to know.

Since it creates a invalid URL, I guess its needs work :)

Well you can still the the page, but the overlay won't show up. If you bookmarked an overlay page that'll break, otherwise it'd be fine.

I applied the patch in #26 in combination with:
patch in #45 from #1468582: Add mobile friendly meta tags to the html.tpl.php
using:
Android 2.3 device (Droid X).
result:
success. Overlay is bypassed.

What about the touch detection from #18?

This is the only good thread I could find discussing this issue, so I thought I'd share a workaround that worked for my simple use case in the hope that it might help others until the more general issue of Drupal-mobile is knocked on the head.

I had a use case where one of our clients wanted to use the overlay for general site navigation (this is a relatively "static" brochureware site), and also utilise some responsive design to handle a mobile display.

ie. each link in the main navigation was to open up in an overlay, unless it was on a mobile device.

We ended up making 2 navigation bars (which ended up being styled differently anyway, so this was convenient for us) and adding the class "overlay-exclude" to each link in the mobile version of the nav. If you check line 546 of overlay-parent.js you'll see that this prevents any link with this class from triggering the overlay.

Maybe, as an alternative to the patch provided in this thread we could do something in the general area of line 546 (for organization) along the lines of:

if($('body').is('.mobile')) return;

And then we work on getting the class "mobile" (or similar, obviously) to be appended to the body by implementing some nice device testing method/library rolled into Core.

Of course, in the meantime you can do something broadly in your theme already by doing something like this (coupled with BYO device detection method :P):

if($('body').is('.mobile')) {
  $('a').addClass('overlay-exclude');
}

I looked at this patch, and it seems a little odd to me that we'd assume functionality based on the width of someone's screen. What if we get it wrong?

For example, I have a relatively new smartphone (because I'm a Luddite and didn't own a smartphone at all until recently). It's an Android phone. And I just tested it; the overlay seems to work fine on it, without any of the problems mentioned above. (Only problem I had was that it was difficult to click the "close" button because it's too small.)

This was with a Drupal Gardens site (easiest way I could access a site with the overlay from my phone), but I don't think Drupal core should be different, or if it is I guess that means there's a bug that is fixable.

It's also worth pointing out that the Overlay module contains an entire system to allow people to disable it (including a message that appears at the top of every page until the user dismisses it; see overlay_disable_message()).

Currently, this message is hidden from everyone except screen reader and keyboard users, because it's there for accessibility reasons. But couldn't we simply un-hide this message for people on small screens also?

It's not a perfect fit, since the preference to turn off the overlay is stored with the user account; whereas in this case storing it as a cookie would probably make more sense (since the user's preference might depend on the device they're using at that particular time). But it's probably good enough, and it seems better than trying to guess what people want based on their screen size.

We are assuming a lot of user experience based on the width of the user's browser. Thats the whole basis behind responsive design. The D8 Mobile initiative wants to implement a whole new user experience for narrow screens and we can't do all that AND keep the overlay. The team has decided that the overlay is an awkward user experience on narrow screens, even if it might technically work on some of them. See mockups at http://groups.drupal.org/node/232653 for what we think is more comfortable.

I'd love a technical review from anyone about whether the current patch is the right way to disable overlay from the client side.

I agree with moshe here, we are not adding the overlay because we think conceptually it does not add the same value as on the desktop. That is being able to quickly move between the site and administrative context. Which is now not something we can effectively communicate on a small-width device. For that reason we want to disable it, it being technically possible or not - is a different point.

Redesigning or replacing the overlay on mobile definitely makes sense.

But that's not what the above patch is doing; instead, it's simply removing it. If we're looking for a fix that's releasable on its own (in general a good idea, and especially since this issue is tagged "needs backport to D7"), then simply killing it with no warning doesn't make sense.

**

A followup: I was slightly wrong about how the overlay behaves on my Android phone. Actually, some of the overlays on my site worked OK (and I could type in the text fields), but others I couldn't. Who knows why. I think the point still stands, though; some day, those browser bugs will be fixed, so if we're treating this as a bug fix (again, how this issue is classified) we have to take into account the fact that screen width doesn't directly determine whether the overlay is functional or not. And therefore until or unless there's an equivalent replacement for small-screen devices, we have to give users a choice (just like we give screen reader users a choice).

What I'm saying, though, is that I don't think it works as an interim solution.

One thing to keep in mind is that we've seen in usability tests that when people are using the overlay they make very frequent use of the close button to go back to where they were before (even in cases where they don't need to; they actually seem to overuse it). So it becomes a very fundamental part of their interaction with the site. I don't think it's a good idea to remove this functionality with no warning and no equivalent replacement, when the person administering the site will expect it to be there.

What I'm suggesting above is an interim solution also, and it would be pretty much the same code, just used in a slightly different way. Maybe I'll work up a quick patch so we can compare.

The attached patch and screenshot shows my suggestion. (Again, this is simply reusing code and user interface elements that are already in place for screen reader users and keyboard navigation users.)

This patch also reuses the code from #26, meaning it's still missing the touch detection (as described in #31) - not sure if that was removed on purpose.

@Moche, fwiw here's my review of the patch in #26:

The patch is split into two parts, the client width detection and the overlay bypass functionality.

I'm basing this review off what I see in firefox, I haven't done any cross browser testing. I had to manually apply this patch since I'm playing with overlay in D7 and there's no "core" directory there so the patch fails.

The client width detection seems pretty straightforward for Firefox, works for me as expected as I resize my browser window. Reading this nice article on quirks mode re-assures me that this implementation "should" be safe everywhere else - http://www.quirksmode.org/mobile/viewports.html

The bypass of the Overlay functionality does appear to be a little bit broken though.

The problem I see is that the current implementation's client width check is only done once as the page is loading (not even waiting for page load) and if the check fails then the entire Overlay API is never loaded. If you load a page while the browser is thinner than 640px and then expand the page the overlay will not appear as you'd expect as an end-user, which is a minor "responsiveness" fail. I say minor because presumably you don't change your browser size every page load and the next time you leave the admin section and come back you will see the overlay once more as you're now using a larger window size.

Another problem with this behaviour which would perhaps be less common but is much more dangerous is that custom modules/themes may be relying on the js methods provided by the Overlay API existing and you could easily trigger unnecessary javascript errors by applying this patch and then break unrelated functionality on a site. For example, I recently did something simple for a client where I called Drupal.overlay.close() whenever the overlay background was clicked while it was still loading to allow users to "escape" from long load times. The same client wanted overlays disabled for mobile devices - we went with adding "overlay-exclude" to our links as I outlined in my earlier comment above but applying this patch would have broken some functionality on that site for some users in a way that would have been really hard to reproduce and debug :(

I propose a simple fix, move the return from where it sits in patch #26 right at the top of overlay-parent.js and move it into Drupal.overlay.eventhandlerOverrideLink which is where you'll find related logic for deciding whether clicking a given link should result in an overlay appearing.

I tested this and it works as expected, but you do have to make sure that the width check comes after the check for "overlay-close" links or you'll find it becomes impossible to close already-open overlays on small screens.

I suppose the main caveat with my approach is that if you visit a direct link to a page with an overlay rendered on a small screen device like "http://example.com/#overlay=admin/appearance", the overlay will still appear. To me that behaviour makes sense as with the current patch or with my modifications there's still no way to get to one of those render-an-overlay links on a mobile device by following links generated by the site - you can only get there if you were to intentionally paste that URL into your browser - but somebody else might see this as a bug so I just wanted to point it out.

I'd roll a patch, but I'm playing with this in D7 and this issue is technically only for D8 which isn't compatible with Aegir on my local dev environment yet :(

In overlay-parent.js just remove the changes made by the current patch and then put this:

  // Never process links other than overlay-close in the overlay when the client width is too small
  // to render the overlay comfortably.
  if(!Drupal.checkWidthBreakpoint()) {
    return;
  }

below this:

  // Close the overlay when the link contains the overlay-close class.
  if ($target.hasClass('overlay-close')) {
    // Clearing the overlay URL fragment will close the overlay.
    $.bbq.removeState('overlay');
    return;
  }

and everything should work just fine :)

Ah, patch in #39 appeared while I was doing my thing offline.

Looks like there's some actual feedback to the user in there as to what is going on and it works with the existing API rather than undermining it.

I like the direction this is going, but I find displaying the overlay when we're well aware that it isn't going to render correctly to be kind of pointless. Is there a good reason to keep it?

It might be nice to offer some configuration of this behaviour. Like a new setting in the appearance/theme config that lets you set a minimum screen size for the overlay or similar. When you think about it, the actual minimum size that the overlay looks "good" at is somewhat dependant on the theme itself - font sizes, page widths, etc.. This thread so far assumes that you're using a relatively "standard" admin theme but that is not always going to be the case.

What if the overlay used media query detection instead of polling the width? #1621594: Media Query Detection

A lot of people in this thread are acting as if we are intending to release Drupal 8 with this code in. That is not the intent of this patch. It's a development crutch.

I'm acting like we're intending to release Drupal 7 with this code, not Drupal 8. That's because this is classified as a bug and the issue has the "needs backport to D7" tag....

Also not totally clear why it's needed as a development crutch for Drupal 8, though. Can't people just turn off the overlay while developing?

Yeah, lets remove D7 from the equation - tag removed.

It is not a crutch for developers as much as a step in the process twoard a new admin user experience for D8. We don't think we can make one big leap there, so we take small steps that may not be all that impressive taken individually. This is pretty common for D8 on all our initiatives.

Both Lewis and Boyjan are saying that Overlay should not be part of the experience for small width browsers. I really don't see the notice proposed in #39 as a step forward.

Pretty sure it would be possible to make a mobile friendly admin theme that would work well with overlay. The problem is more to do with the minimum width required for the common admin themes to display properly in an overlay rather than a device specific incompatibility.

Does a device having touch compatibility break the overlay somehow? if it doesn't we should update the title of this thread.

in response to #43 - why is this ticket flagged as major if we're not planning to commit or otherwise act on anything discussed here?

1. OK, I moved the patch from #39 over to #1655098: Overlay doesn't work well on touch devices or widths less than 960px - give people an easy way to turn it off (with the "needs backport to D7" tag there, since this is still a bug) and we'll let this issue focus only on entirely removing the overlay on mobile for Drupal 8.

2. I rerolled the patch from #26 taking into account @thedavidmeister's feedback in #40. It actually seemed to me like the best place to do this would be in Drupal.behaviors.overlayParent (before eventhandlerOverrideLink method is even bound), so I put it there instead... It seems to work well. With the patch, URLs like http://example.com/#overlay=admin/appearance will now work if you go to them directly; by moving the code a little higher in the function we could prevent that though.

Personally I think it might make sense to postpone this issue, though? It's the kind of change that could easily go in later (including after D8 code freeze), and by that point we'll actually know if we have an alternative mobile experience in place and if so what it looks like.

Agreed, need to find a global strategy for this kind of things: #1252178: Add Modernizr to core hopefully we'll make some progress on that next month.

I just tested #48 and it works well for me and addresses all the concerns that I raised earlier.

Here's a backport for D7.

The core committers should decide whether to postpone this or not. There is significant support for the patch at #48 from Drupal's UX folks, even before an alternate solution is committed. The overlay experience was simply not designed for small screens. See comments from lewisnyman, rupl, bojhan, ...

Please ignore the patch in #50. I think we have agreed not to change D7 at this point. Folks are welcome to use it on their own sites.

Instead of checking the window size and then disabling the overlay, why don't we check to see if a particular media query is active then disable or enable the overlay?

See #1621594: Media Query Detection

Reuploading #48 as per #51, so that bot doesn't keep downgrading the status.

Setting this to "needs review" rather than #51's RTBC, because from what I can tell, the patch doesn't match the issue title: I see nothing in the patch that has anything to do with touch devices, so if we want to RTBC this patch, please also change the issue title accordingly, and then we can open a new issue for touch devices.

Re #54: #43 has one answer. Also, there's no conclusion yet to #1621594: Media Query Detection. Given that, do we really want this postponed on that?

More humble title

Committed to 8.x. Let's get some more progress on the mobile stuff now.

Yeah, patch in #50 was provided for personal use if you're running D7 and want a fix without waiting until the next major version. Patch fails because this issue is marked for D8, not because the patch is bad. Works fine through drush make.

I applied the backport for D7 and see that the overlay is still presented to me on my iphone 4s. As you are probably aware, the overlay does not work, as you try to scroll down the overlay, the page scrolls back up to the top. I don't think screen resolution is the best method to decide who gets an overlay and who doesn't. it might be best served to check for the browser's useragent.

...or #1252178: Add Modernizr to core

I've extended this issue with #1829326: Convert Overlay to leverage core breakpoints and media queries to determine its presence and styling.

Issue tags aren't Twitter hashtags. :-)

Thanks, removing from UX sprint now.

The same fix will work for D7 too. (Tested on 7.50)

The patch is a simple backport to D7.

The patch in #65 doesn't appear to have an effect in 7.50 - the overlay is still presented and not scroll-able. Iphone 6.

@pendashteh - Thanks for looking at this.

Setting back to needs work based on @kruser's feedback in #68