UX testing results are in, see comment #108.

This idea was raised in #659488: Properly test the overlay to determine if it belongs in core or contrib (originally by Owen Barton) as a simpler way to potentially solve some of the "loss of context" issues that the overlay is also trying to solve. This would be for users who do not use the overlay (wherever and however the module ends up).

We are moving it here so it can get its own discussion. Below I have copied some of the relevant discussion from the other issue:

#178
Owen Barton - January 5, 2010 - 13:37

I have just read the entire thread, and wanted to add some analysis and 2 proposals that I think address the original UX problems in a much simpler and as effective way.

So - if we compare browsing with the overlay, to browsing Seven without it - the actual visual and functional differences are really quite small:
A: There is a large black border that faintly shows the original page you were on before you hit the admin area. Many people have noted that the underlying page is barely noticeable, and pretty much everyone was unanomous that it was too distracting when it was lighter/transparent.
B: There is an "X" at the top of the page that returns you to the original page when you click it.

It seems to me that (A) adds little value - it is too dark to provide much context (you can't exactly refer to the content on that page while in the admin area), and is too distracting as a border on a large admin window when made lighter. (B) does provide a useful function, but doesn't really provide context either. While proper A/B user testing would prove it for sure, it seems very unlikely to me that these 2 small visual/functional changes are going to radically improve the UX. Especially when you consider the volume and complexity of the code needed to support these 2 changes it seems that there has to be a better option.

So here are my 2 proposals (we could do either or both):

1: Make regular admin pages non-overlay, but provide a way for users to easily get back to the last non-admin page they were on after they are done with their admin tasks. This could use an "X" link or it could use a "Back to History of the Drupal community 2008-2010" link, perhaps floated right on the same line as the breadcrumbs. The latter, together with the new admin theme would seem to provide the context to users in an equally or more effectively as the black border. This could be implemented with the adapting the destination URL parameter (or a new parameter) so that url() passes it unchanged on admin paths, or using a (tab/window sensitive) session value - either way should be very simple.

2: Use a more traditional modal popup for lightweight tasks. By lightweight tasks I mean "single hit" tasks that generally you only ever expect a single route through, normally a single form, before returning to the original page. We could have a simple path textarea for site builders to configure where these show up - by default they would probably be the forms accessed via the inline hover links - node/*/edit, admin/build/block/*/edit etc, as well as */confirm. Any links inside the popup would either open in the same page, or in a new (actual) window - never in the modal popup. Most of the time the popup could have a much smaller window, which should (together with the fact that the user is only 1 step away from their current page) allow the context to be much more apparent. Because we can use a more traditional popup metaphor we probably do not need to gray out the background. This could use the core of the overlay code, but could probably drop much of the AJAX/URL complexity we use to keep admin links loading in the overlay.

#270
David_Rothstein - April 28, 2010 - 09:31

I think that Owen Barton's proposal in #178 is an interesting alternative. It definitely tries to address some of the same issues as the overlay, and in a much simpler way. I'm not sure it provides context quite as strongly as the overlay does, but that would be something interesting to test.

Here is a patch which provides a quick and dirty implementation of Owen's idea - just trying to get it towards something testable. This only makes sense right now if you turn the overlay off and leave the administrative theme on (including for content creation), but if you do that it should be somewhat testable. For a real implementation, #669510: Merge administration theme with hook_admin_paths() would probably be a requirement for this to make sense.

This would probably need some CSS work (and resolving the conflict with the breadcrumbs) in order to be more usefully testable - see attached screenshot :) Notice, by the way, that Owen's idea can sort of be seen as a more interesting and reasonable alternative to the infamous "Back to the live site" link from the original non-overlay Seven implementation (which was thankfully removed in #546694: "Back to the live site" is misleading and incorrect and #548806: No breadcrumbs in Seven). But this version of it might make a fair amount more sense.
Attachment Size Status Test result Operations
Attachment Size Status Test result Operations
overlay-alternative-659488-269.patchreview 1.94 KB Ignored None None
back-to-previous-page-link.png 103.88 KB Ignored None None

#273
EvanDonovan - April 28, 2010 - 13:52

@David_Rothstein: Oooh, interesting. I missed that. That might cool, even for people who don't ever want overlay enabled. Will try to test soon.

#274
Cliff - April 28, 2010 - 22:02

@David_Rothstein, I agree that your approach would work. In fact, from a design standpoint, it makes more sense than an overlay. With an overlay, all pages with the same border could look the same, or at least similar enough for the user to goof. And that really won't show up as a problem until you have a lot of users working with it on rather large sites — in other words, sites with many pages that are similar in overall format.

With your approach (if I understand it correctly), there is a unique identifier in the link text pointing back to the page on which this work is being done: The title of that page. It's a simpler fix at all levels, and it's accessible.

If that approach were adopted, would Overlay be removed from core because it would no longer be necessary? Or would it simply be turned off by default?

#277
cha0s - April 29, 2010 - 05:00

I have a feeling everyone's gonna come out of the woodwork against this one, but I had the idea so I made it happen and figured I'd share it.

This patch adds a new function, drupal_get_breadcrumb_back(), which returns the path that David ascertains with his current patch. That code has been updated in a couple ways; it doesn't act if the user is anonymous, and only adds the back link if the back path is different than the front page (as that would be redundant with 'Home').

So, I'll let the image/patch tell you the rest.
Attachment Size Status Test result Operations
Attachment Size Status Test result Operations
breadcrumb_back.png 66.21 KB Ignored None None
659488.patchreview 4.78 KB Ignored None None

#278
cha0s - April 29, 2010 - 05:06

Oops, copy paste is bad mmkayy...
Attachment Size Status Test result Operations
Attachment Size Status Test result Operations
659488.patchreview 4.46 KB Ignored None None

#279
kika - April 29, 2010 - 10:02

Great proposal, but:

1) I think it warrants a separate issue, this thread is way too lengthy to work on

2) Note that there is a similar UI proposal in the works what works other way around -- a special link getting _back_to_the_admin_page_ for "Demonstrate block regions" functionality. #655416: "Demonstrate block regions" bugs with regions, navigation, overlay and the mockup: http://img.skitch.com/20100421-xdnrpebeg4ty8g6ybyk1178cyp.jpg

This means the context-aware backlink needs to be nicely abstracted so in different scenarios different link content can be injected. Also, it should be bolted to permanent place in the page (likely docked to the menubar) and work both in admin and non-admin pages.

#280
Owen Barton - April 29, 2010 - 12:14

Agreed that we should move this into a different issue, and that we should have some reasonable abstraction.

I also think that we should probably steer clear of sessions, unless someone has a way of ensuring this works properly across multiple tabs. I was thinking more along the lines of a persistent querystring parameter (like destination=), or possibly a context style URL (e.g. news/drupal-7-released/admin/build/modules), where everything in front of admin is removed from $_GET['q'] and stashed somewhere for later reference, in the same way the language prefixes work.

In terms of UI, the "back" (<<) breadcrumbs are an interesting idea and do make sense to me. I am not sure if that is a bit to unusual from a UX point of view though - I was thinking more of just having a normal link, positioned vertically in line with the breadcrumbs, but floated all the way to the right.

#281
kika - April 29, 2010 - 12:33

For me, messing with breadcrumbs does not feel right and UX will start wildly differ in different themes. I'd go for a special (centered) docked element as suggested by yoroy http://img.skitch.com/20100421-xdnrpebeg4ty8g6ybyk1178cyp.jpg

#282
Owen Barton - April 29, 2010 - 17:32

Yeah - that is more what I was thinking of.

#283
EvanDonovan - April 29, 2010 - 18:05

I really like yoroy's design mockup. That might please everyone. We should move to another issue if we're going to have actual code, though.

#284
cha0s - April 29, 2010 - 21:00

Yeah, that's pretty sexy. My only concern is, is it too late to get in? Are we pursuing this as an alternative to overlay or introducing this in addition to the overlay?

#285
kika - April 30, 2010 - 10:57

Have we not been in that backlink-land? #546694: "Back to the live site" is misleading and incorrect #548806: No breadcrumbs in Seven ?

#286
Owen Barton - April 30, 2010 - 13:04

Actually, reading those I think what is discussed here is different, but I suspect closer to what Mark Boulton and co were aiming for. It looks to me that the mockups showed a "Back to front page" link and someone added that hard coded (which was pointless, as home is already in the breadcrumbs), but I think the intention was for that to be context sensitive to the last non-admin page you were on, which is what we are looking at here.

Either way, the main point is that the Overlay was intended to give the user a context about where they "are" on the site, but currently it just gives a thin dark border that makes the underlying site barely visible (especially anything specific to the actual page you were on), and an "X" button that takes them back to the last non-admin page they were on. My feeling is that a well placed "Back to "" link as described above would provide this context better than the current Overlay, as well as the same functionality as that "X". It could replace the overlay completely (we could then use modal dialogs for temporal interactions, such as confirmations - which I think is a more valuable use overall), but even if the Overlay stays at very least this link would at least help fulfill the underlying user requirement of context that the overlay is (IMHO) failing to provide.

#287
kika - April 30, 2010 - 14:12

What is missing to have an actual code? The backlink-storing is already in place, we just need a permanent place for backlink in templates and styling (I can help with the latter). Where this stuff should live at first place, file-wise? If it is bolted to menubar (toolbar is optional) then the presentation should be in the same place where menubar? How to make it work for #655416: "Demonstrate block regions" bugs with regions, navigation, overlay ? Does it need a special alter hook or render altering will do?

CommentFileSizeAuthor
#147 core-overlayslayer-ux-approved-787896-147.patch7.24 KBjp.stacey
#139 core-overlayslayer-ux-approved-787896-139.patch7.24 KBnod_
#145 overlayslayer-front.png35.56 KBnod_
#145 overlayslayer-admin-back-link.png33.62 KBnod_
#145 overlayslayer-admin-no-back-link.png32.37 KBnod_
#139 interdiff.txt420 bytesnod_
#138 core-overlayslayer-ux-approved-787896-138.patch7.21 KBnod_
#138 interdiff.txt972 bytesnod_
#137 interdiff.txt1.68 KBnod_
#137 core-overlayslayer-ux-approved-787896-137.patch6.48 KBnod_
#132 core-overlayslayer-ux-approved-787896-132.patch6.61 KBnod_
#132 interdiff.txt832 bytesnod_
#126 interdiff.txt608 bytesnod_
#126 core-overlayslayer-ux-approved-787896-126.patch6.42 KBnod_
#124 core-overlayslayer-ux-approved-787896-123.patch6.42 KBnod_
#122 interdiff.txt5.92 KBnod_
#122 core-overlayslayer-ux-approved-787896-121.patch5.87 KBnod_
#111 interdiff.txt1.43 KBnod_
#111 core-overlayslayer-ux-approved.patch5.16 KBnod_
#65 superoverlayslayer-front.jpg23.4 KBnod_
#65 superoverlayslayer-back.jpg28.65 KBnod_
#65 superoverlayslayer-back-nofront.jpg27.76 KBnod_
#94 core-overlayslayer-1.patch4.28 KBnod_
#88 google-chrome-back-history.png9.63 KBRobLoach
#64 core-overlayslayer.patch4.28 KBnod_
#70 d7-back-link.png9.43 KBDavid_Rothstein
#61 overlayslayer-only.jpg127.63 KBnod_
#61 overlayslayer-with-tour.jpg108.22 KBnod_
#60 core-js-escapeAdmin-787896-60.patch4.89 KBnod_
#59 core-js-escapeAdmin-787896-59.patch4.89 KBnod_
#58 core-js-escapeAdmin-787896-58.patch4.73 KBnod_
#54 core-js-escapeAdmin-787896-54.patch4.7 KBnod_
#53 Screen Shot 2013-09-27 at 19.06.01.png146.26 KBmcjim
#53 Screen Shot 2013-09-27 at 19.05.41.png150.3 KBmcjim
#51 Screen Shot 2013-09-27 at 17.05.08.png203.38 KBmcjim
#46 core-js-escapeAdmin-787896-46.patch4.29 KBnod_
#30 drupal.breadcrumb-back.30.patch4.27 KBsun
#22 787896.patch2.48 KBcha0s
#19 787896.patch2.42 KBcha0s
#15 787896.patch2.63 KBcha0s
#1 659488_0.patch4.46 KBDavid_Rothstein
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

David_Rothstein’s picture

Status: Active » Needs work
FileSize
4.46 KB

Reposting the latest patch from @cha0s on that issue.

Marking "needs work" since we seemed to agree that rather than making this inline with the breadcrumbs, it would be better to have it styled in the same way as what is being proposed at (and whatever eventually comes out of) this issue: #655416: "Demonstrate block regions" bugs with regions, navigation, overlay

Owen also suggested (see above) that perhaps we don't want to use sessions since that could cause issues when the user has multiple browser tabs open. That's an interesting point, although given that the point here is to help maintain context it's kind of tricky situation no matter what. If you have two browser tabs open (to different non-admin pages) then go do an administrative task in one, then another administrative task in the other, then switch back to the first... well, your "context" (from a mental standpoint) is probably lost anyway :) In some ways, using $_SESSION might actually be sensible in that case, since it guarantees that the link you see will always take you back to the last non-administrative thing that you were doing (regardless of what browser tab you were doing it in). However, visually it would look a bit strange I guess.

sun’s picture

This makes sense. Though we need proper getter/setter functions.

yoroy’s picture

Do we really have to replicate the browsers 'Back'-button functionality? #655416: "Demonstrate block regions" bugs with regions, navigation, overlay is a pretty special case and I doubt this approach will help the 'keep context' problem overlay wants to solve.

kika’s picture

> Do we really have to replicate the browsers 'Back'-button functionality

As far as I understand, "Back to x"-button is functionally equivalent to closing overlay with (x) button. It is not the same as "Back" when you happen to be deeper in admin pages.

casey’s picture

Subscribing.

kika’s picture

joachim’s picture

Doesn't work on Garland, which presumably overrides the theme function.

Also, I find it looks a bit strange to have the the 'home' link surrounded by << >> -- looks like it's quoted!

PS. But in general, this is great :)

eigentor’s picture

Great stuff.
This is what the overlay is about, and it really makes sense to built up an alternative that keeps the most important part (poorly disguised subscribe post).

EvanDonovan’s picture

kika in #4 is correct, as far as I understand the proposal. This is not just the same as the back button, since you can be multiple levels deep in the admin interface.

IMO, although this is not backed up with testing (yet?), this would work better for most administrative tasks, whereas the overlay would work better for quick tasks triggered by the clicking of contextual links.

Perhaps we could have overlay in some form for contextual links (and adding/editing content?), but use this for pages like blocks and modules, which I really did *not* want to see in the overlay. That might be a compromise that would satisfy most people, as long as the accessibility issues with Overlay can be resolved.

(Subscribing... :) )

joachim’s picture

> This is not just the same as the back button, since you can be multiple levels deep in the admin interface.

That's correct. I tried the patch on Stark theme, and starting from node/x (ie some random 'front-facing' content), went into an admin page at random, went several levels deep. The 'back to ...' link remained constant at the last non-admin page I was on.

cha0s’s picture

Anyone got code or ideas how to style that tabby thing from http://img.skitch.com/20100421-xdnrpebeg4ty8g6ybyk1178cyp.jpg ?

casey’s picture

Have a look at #787940: Generic approach for position:fixed elements like Toolbar, tableHeader

With that patch this will be much easier.

yoroy’s picture

Excuse my simple brain. Understood that this would be a more smartypants back button.
Can we keep the visual design discussion in #655416: "Demonstrate block regions" bugs with regions, navigation, overlay though?

Any working patch here should focus on the abstracted functionality, but keep #655416: "Demonstrate block regions" bugs with regions, navigation, overlay in the loop if you do.

Owen Barton’s picture

Thanks for opening this issue David.

Before we get too far, I think we should carefully consider the use of session vs. non-session solutions here. Users expect separate tabs to be separate, they never expect an action (with the exception of a form submit or a clearly labeled action link) in tab A to have an effect or side effect in tab B. In my experience when this happens this almost always results in confusion. I can think of several situations where I would want to go into 2 or more simultaneous admin activities, yet still want to go back to the last content page I was on in each case - not just the one I happened to come from last. Of course, there may be some method to prevent (or at least limit) side effects with sessions, or we could use some link state mechanism as I described previously.

cha0s’s picture

FileSize
2.63 KB

I changed a few things in this new patch, notably removing the front-end stuff from before.

The function is invoked in the preprocess hook as before... due to the way this is implemented, it must be called on every page or else context could be lost. It's not the best in the world but hey.

Owen, I'm interested in your 'link state mechanism'... I studied many of the issues surrounding this in detail and didn't find that. Please share.

yoroy’s picture

Status: Needs work » Needs review

status for testbot

Owen Barton’s picture

Status: Needs review » Needs work

It's in the description...

#280
Owen Barton - April 29, 2010 - 12:14
...
I also think that we should probably steer clear of sessions, unless someone has a way of ensuring this works properly across multiple tabs. I was thinking more along the lines of a persistent querystring parameter (like destination=), or possibly a context style URL (e.g. news/drupal-7-released/admin/build/modules), where everything in front of admin is removed from $_GET['q'] and stashed somewhere for later reference, in the same way the language prefixes work.

The latter one has the advantage of being essentially the same as the way overlay URLs work, except that the '#' is replaced with a '/'.

cha0s’s picture

Ahh interesting... I'm going to give this a shot very soon.

cha0s’s picture

Status: Needs work » Needs review
FileSize
2.42 KB

Check out this craziness.

Status: Needs review » Needs work

The last submitted patch, 787896.patch, failed testing.

cha0s’s picture

Hm, strange tests to fail.

cha0s’s picture

Status: Needs work » Needs review
FileSize
2.48 KB

I might be asking for a smackdown, but I think we actually need to rewrite $_GET['q'] to complete the deception that we're on an admin page.

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

The last submitted patch, 787896.patch, failed testing.

mgifford’s picture

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

#22: 787896.patch queued for re-testing.

mgifford’s picture

That last patch applies nicely in my sandbox. However, I'm still having trouble tracking down where I would see it expressed. I did see a screenshot in an IRC discussion, but it must be that drupal_get_last_non_admin_page() is written in somewhere else to produce the link back.

Anyways, I do like the idea of this patch.

Status: Needs review » Needs work

The last submitted patch, 787896.patch, failed testing.

cha0s’s picture

Well, this isn't supposed to be the actual themed part. This is just supposed to be the logic.

Regardless, it's starting to look like everyone has forgotten about this issue...

webchick’s picture

Version: 7.x-dev » 8.x-dev
webchick’s picture

Oh, hrm. I guess I should read an issue before just blatantly bumping it to 8.x since it's a feature that changes the UI after UI freeze, and doesn't fix a critical in the process. Still, though. Isn't it a bit late for this? It took us some 150 replies over at #655416: "Demonstrate block regions" bugs with regions, navigation, overlay to come to consensus about how to approach that problem, and I'd like to get a beta out the door here in a couple of weeks. This could be added by contrib for this release, no?

sun’s picture

Version: 8.x-dev » 7.x-dev
Status: Needs work » Needs review
FileSize
4.27 KB

Since Overlay is still pretty much unusable for me, this would still be the proper solution.

Moving back to 7.x + attaching a cleaned up and revised patch based on the David's original one for discussion and review. Alternatively, to make this code possible to work from contrib, I think we'd have to bring some flexibility into theme_breadcrumb(), basically to allow for an optional prefix/suffix, or similar, generally speaking: slightly change it so that it respects more stuff being passed to it.

Status: Needs review » Needs work

The last submitted patch, drupal.breadcrumb-back.30.patch, failed testing.

sun’s picture

woah, holy cow. I mistakenly had this patch still applied on my dev site, and thus stumbled over the link while manually testing something else. Awesome idea + experience, totally rocks.

Everett Zufelt’s picture

Version: 7.x-dev » 8.x-dev

I'd love to see this feature implemented in D8 for those users who have the Overlay disabled.

Everett Zufelt’s picture

Note: I'd also love to see this implemented as a D7 contrib module if possible.

Other than positioning and styling, is there anything that would prevent the functionality from being reproduced as a contrib module?

EvanDonovan’s picture

@Everett: I think it could be replicated in D7 contrib via an implementation of hook_preprocess_breadcrumb and theme_breadcrumb in a contributed module or theme. The back link for the breadcrumb would probably have to be set in hook_init() or something, though.

David_Rothstein’s picture

Right, this shouldn't be too hard to do as a contrib module if there is interest.

Actually, I don't even think you'd need hook_init(), would you? You could probably just do everything in the breadcrumb theming and preprocess functions. (After all, that's more or less the only place in core that this patch touches.)

sun’s picture

Speaking of, implementing this as an option (among some others) is one of the stated goals of http://drupal.org/project/breadcrumb

EvanDonovan’s picture

@sun: Last commit 6 years ago by Matt Westgate? So I take it this is an old module that you have commandeered? :) I was aware of Custom Breadcrumb, but I didn't know if this would be in scope for a 7.x version...guess maybe Breadcrumb module will supersede all the many breadcrumb modules.

@David_Rothstein: I thought #'s 22 & 30 made it sound like something more needed to be changed. But I think you're right actually.

effulgentsia’s picture

I haven't read this entire issue yet, but it got brought up in #1889150-72: Simplify overlay look, *only use for contextual operations* as something that might be relevant if/when that issue lands, since that issue is currently proposing to limit Overlay use to contextual operations only, in which case the Overlay close button will cease to be the standard core way of returning to the last non-admin page.

David_Rothstein’s picture

I closed #2098713: Overlay alternative. as a duplicate, but it had the interesting idea of doing this via sessionStorage (and an initial patch that did so).

I think that would deal with the problem of maintaining separation between multiple web browser tabs that was discussed above. Should discuss the pros and cons compared to the other way that was discussed here (putting the info as a parameter in the URL), but overall it seems pretty good.

nod_’s picture

also. the use of session storage totally solves the tabs issue.

edit: for some reason didn't see you said that in last comment sorry :p

cha0s’s picture

Yup, sessionStorage is way cleaner than whatever hax I was attempting there. Nice to see this getting another go around ;)

nod_’s picture

Assigned: Unassigned » nod_

I'm intending this patch to replace totally the overlay and go ahead with #2088121: Remove Overlay once we're RTBC here so that we don't have to maintain 1k lines of JS dealing with iframes and making ugly URLs.

I'll work on this at the sprint today. I'll try baking that patch in the toolbar directly, if you guys have a better idea on where to put it I'm open to it. As you can see in the other issue it's pretty ridiculous how little code is needed.

cha0s’s picture

Assigned: nod_ » Unassigned

Is it finally happening? Is Overlay finally dying? Oh, what a joyous day! nod_, you're my hero <3

Gábor Hojtsy’s picture

I think sessionstorage sounds like the best option. Carrying the back URL around in the main URL sounds dangerous. Eg. bookmarking the URL would give you inconsistent behaviour later on, and we may likely loose that URL tack-on in various ways, eg. clicking links in the toolbar would need to have it, breadcrumbs, all kinds of nav links, form submits, etc.

nod_’s picture

Status: Needs work » Needs review
FileSize
4.29 KB

Patch that looks ok, probably stuff to change but people can try it out.

It's integrated in the toolbar, not sure that's where we want it but if contrib needs it, it's trivial to implement.

If you hit the escape key on an admin page and you have the "back to site" button visible, you'll be redirected :)

nod_’s picture

yoroy’s picture

A non-trivial change, so screenshot would be nice. :-)

nod_’s picture

Status: Needs review » Needs work

The last submitted patch, core-js-escapeAdmin-787896-46.patch, failed testing.

mcjim’s picture

joachim’s picture

Thanks!

IIRC the toolbar has something on the far right when it's not in sidebar mode. How would it work with that?

mcjim’s picture

Horizontal toolbar without this patch:

Without "back to link" horizontal toolbar

Horizontal toolbar with this patch:

With back to link, horizontal toolbar

nod_’s picture

Status: Needs work » Needs review
FileSize
4.7 KB

Not blue anymore but meh. there is an icon now and RTL should work.

cha0s’s picture

Just want to air it out, does it seem like the 'back to site' is close enough to the log out button that it my cause problems with misclicking?

Status: Needs review » Needs work

The last submitted patch, core-js-escapeAdmin-787896-54.patch, failed testing.

nod_’s picture

Ummm I'm not seeing the back to site link anywhere close to the logout button, can you explain?

Also anyone knows how to get around the test failures? Problem is using hook_admin_path (because I use path_is_admin()) during upgrade.

nod_’s picture

Status: Needs work » Needs review
FileSize
4.73 KB

trying to use
+ 'currentPathIsAdmin' => MAINTENANCE_MODE !== 'update' ? path_is_admin($current_path) : TRUE,

Open to better ideas.

nod_’s picture

now with less warnings.

nod_’s picture

Sorry for patch spam, the 2 previous ones didn't actually work.

Making ifs in the morning is not good for me apparently :p

nod_’s picture

screens below

(edit) remove inline images, not up to date anymore.

Wim Leers’s picture

The icons for "Tour" and "Back to site" don't align vertically.

Leaving at NR for more feedback.

yoroy’s picture

I'm sceptical about having a 'Back' button at the top right. In LTR world, back buttons are the most left ones with arrows or triangles pointing to the left.

Why not insert it before the 'Home' icon? -> That would be jarring because menu items would move back and forth.

I can even imagine swapping out the Home icon with the back button.

To me the blue treatment works better, I think the latest design and placement are too subtle, don't do enough to "replace overlay" in that sense.

nod_’s picture

Title: Add a link so that administrators can return to their most recently visited non-admin page » Add a "Back to..." link so that administrators can return to their most recently visited non-admin page
FileSize
4.28 KB

Oh! that's brilliant. Taking over the Home button and making it as a back to site link feels really good to use. And it makes the code simpler still.

About replacing the overlay there is also the fact that all the new users of Drupal and D8 have no idea the overlay existed before :þ

(edit) to test the patch you need to disable the overlay for your user (or uninstall it altogether).

nod_’s picture

Title: Add a link so that administrators can return to their most recently visited non-admin page » Add a "Back to..." link so that administrators can return to their most recently visited non-admin page
Category: task » feature
Priority: Major » Normal
FileSize
27.76 KB
28.65 KB
23.4 KB

See #94
Any non-admin page:

Going in an admin page from a non-admin page:

Going directly to an admin page or opening admin page in new tab:

webchick’s picture

That looks effing bad-ass.

geerlingguy’s picture

I really like it, too; and I'm used to hitting the home button to "escape from the admin area", so this is a major UX win in my opinion. +1 on replacing home link.

Gábor Hojtsy’s picture

Yeah, bad-ass!

nod_’s picture

In case it makes it, Bojhan and yoroy should be credited.

David_Rothstein’s picture

FileSize
9.43 KB
  1. About replacing the overlay there is also the fact that all the new users of Drupal and D8 have no idea the overlay existed before :þ

    I think what he meant is that we know from experience (and UX testing) that users immediately see and understand the visual metaphor of the overlay; they gravitate towards the close button and know how to use it. To have a similar effect, this has to be easily findable and understandable.

  2. I'm a little skeptical of replacing the Home link because (a) often I really do want to go home and with this patch there won't be a link to do that at all, and (b) don't people get very used to going to the top left of the screen and clicking to go home? To have that area do different things in different situations could be confusing.
  3. "Back to site" doesn't provide much context - or accurate context (I'm still on the site the entire time). That's why that wording was eventually rejected for Drupal 7 and why the earlier patches here used "Back to [page title]" instead. I can see that running into problems with large page titles though.
  4. Instead of a new design, can't we reuse or enhance the one that's already in core? I couldn't do a screenshot of this in Drupal 8 since the block demo page seems to be completely broken, but if that were working it would be there already, and here's what it looks like in Drupal 7:

    d7-back-link.png

    Simply building off that design (and enhancing it a bit, e.g. adding an arrow) could work? And it would allow both of the previous points to be addressed.

  5. Implementation-wise, I think it would be preferable to not have this be part of the Toolbar module. Not everyone using the toolbar will want this feature (for example, you probably wouldn't want this and the overlay at the same time), and conversely, some people not using the Toolbar module would want it too (example: Admin Menu). It would make more sense as an independent module to me.
nod_’s picture

Title: Add a link so that administrators can return to their most recently visited non-admin page » Add a "Back to..." link so that administrators can return to their most recently visited non-admin page

1. That wasn't a really serious comment from me. I got what he meant.

2. That's fair enough. At least the icon and wording is different so it's possible to understand that the behaviour *might* be different between Back to site and Home.

3. Also true. "Back to public site", "Exit administration" or similar might be better? I don't have a strong opinion on this. I'm happy to change it to whatever suits best.

4. Like the overlay warning message to turn it off? It's going to be on pretty much all the admin pages. I can give you one data point that is that it would get in my way and that I wouldn't like it. Toolbar + tray + ad-hoc message bar feels too much to me.

5. I considered it, but then it's one module for pretty much nothing. The code is trivial, what's really important is the drupalSettings.currentPathIsAdmin variable, the rest is like 5 lines of JS. Contrib can even reuse the JS file from the toolbar module as long as they use the same data- attribute. (edit) also this patch suppose that when it gets in, the overlay is removed from core.

nod_’s picture

Title: Add a "Back to..." link so that administrators can return to their most recently visited non-admin page » Add a link so that administrators can return to their most recently visited non-admin page

title.

Bojhan’s picture

Title: Add a "Back to..." link so that administrators can return to their most recently visited non-admin page » Add a link so that administrators can return to their most recently visited non-admin page

I applaud nod_ his effort to get this issue rolling again. I like this issue, because it shows an informed UX discussion.

I have always been a strong advocate of the Overlay, because it was able to bridge the gap that people feel disconnected with the thing they are working on (the front end) and allow for people to reset/jump back quickly. The affordance was clear, and people were able to understand its use easily (usability testing proven this). It might be a rather different model than most CMS's employ, but I think its been a good pattern so far to battle this problem.

nod_ approached me a week or two before Drupalcon to see what we can do to resolve the situation. I expressed clearly why it was there, and why triggering only from contextual links would cause a very fragmented experience. At Drupalcon we revisited this solution, it is still a viable one when executed correctly. The issue we found with it previously that it just simply wasn't discoverable enough to drive the same behaviour as the full overlay does.

David is right, that we should explore ways to get close to the same behaviour and this is done in two steps 1) exploring the right visual alternatives to trigger the (going back to my front-end context), 2) validating this with actual users. Given the significance of this feature, we should follow the same path as we did with the Toolbar of usability testing it - I'd also shows that we are serious about UX and don't make jumps based on feelings.

Onto David's points:

2) Agreed, the design from #53 has the same kind of pattern Twitter uses and its highly noticeable. I am not sure I like it, in combination with all the other top right icons. But frankly, those are all a little weird. The edit inline, tour, and that responsive toolbar thingy all employ different "tray" and "modus" patterns. We gotta rethink that, if we want to do this.

3) We did think about page titles, the question is how representative that is? Any ideas? We can abbreviate long titles, but we gotta make sure the "contextual parts" remains.

4) I have serious doubts about the effectiveness of that pattern. When we did test it, it was often not noticed. I think we need a new pattern here, and sadly it requires a bit more redesigning of the toolbar for it to fit than initially anticipated. Because do we make this work on mobile too?

5) It can be a separate module, if we are removing the overlay. Its not completely weird to make it a module.

Making sure that its discoverable enough to trigger the same behaviour is key, if its not. Then we simply cannot remove the Overlay without causing a critical regression. When we arrive at a good design, we can hopefully scramble the resources to usability test it - although that part worries me a bit, as our UX testing resources are small.

rachel_norfolk’s picture

Been thinking about the code being in the Toolbar module and having a pink hair moment...

I really like how this works; it totally makes sense to me. I was wondering, however, what would happen if someone disabled the Toolbar module to replace it with something better? (Assuming some clever soul creates something better in the future - I'm going to say as an example the "gesture admin menu")

The functionality of the Back to Site link strikes me as totally fundamental to Drupal. Could we do something like the following scenario?

  1. Base code for managing the Back to Site link is kept very much as a core piece of code and exposed as an interface*;
  2. The Toolbar module implements that interface just like it is done above;
  3. Other modules that want to introduce a fancy-pants admin menu can implement the Back to Site link in any way they think works for them;
  4. There might even be a way to make Overlay work using the interface I've described above, so we can keep it as an optional module. (Definitely not enabled in the default profiles, though.)

*(I'm using the English meaning of Interface rather than a php code meaning here.)

rachel_norfolk’s picture

erm - I guess now I understand it better, the code *is* a kind of an interface that can be implemented by any admin menu implementation. We would just need to document how...

tkoleary’s picture

Title: Add a "Back to..." link so that administrators can return to their most recently visited non-admin page » Add a link so that administrators can return to their most recently visited non-admin page

@nod @yoroy @bojhan @DavidRothstein

I have absolutely no problem with "back to site". Obviously we should test it (as with everything), and 'till we do more discussion is wasted since we may be debating moot points.

For my part, I think this patch is awesome.

webchick’s picture

So, everyone likes the patch. Who's planning to run the UX test for this? Bojhan, is that something you're going to put together?

Bojhan’s picture

I am not putting this together. I do not have the resources to test this atm, given that dcmistry and lisarex are not available anymore I have no idea how to make this happen. We usually collaborated on activities like this, doing this alone - is rather hard. I will bring it up with the UX meetup today.

nod_’s picture

Ok just to make sure, right now, regardless of the patch there is no way to pass the core usability gate?

webchick’s picture

Yeah, I don't think that's acceptable. We can't block all major-ish UX-affecting patches until some future time when we hope there will be volunteers willing and able to run formal UX studies. We either need the UX team to define a light-weight, grassroots process that anyone can run with, or we need to amend the gate to reflect reality.

Bojhan’s picture

So unless, very bluntly unless the UX team does the work - you remove the gate? I think this pretty much explains why gates never worked.

There are hundreds of online resources how to do usability testing, we have always supported people who wanted to run these studies to get major changes in (test script development, analysis, etc). I don't think it's reasonable to expect the UX-team to do all testing, especially when its on something that has tested very well.

webchick’s picture

No, I said that unless the UX team that defines a low-weight process that anyone can follow on their own patches, then it can't be a gate. If the process requires a usability professional to execute, we can't keep it as a gate.

Bojhan’s picture

As I already mentioned we are happy to help with study development and analysis. There are hundreds of resources on doing guerilla usability testing. There is no reason for us to replicate that. However doing a user test, is not a 1-2-3 thing - just like writing documentation, making something performant - it is a task that takes time.

lisarex’s picture

"given that dcmistry and lisarex are not available anymore" > I'm still available, but my availability is still very limited.

I'm going to try to fit in a quick UX study with people around me or remotely but I could use everyone's help finding participants: people who are familiar with editing content but aren't aware of this issue / patch :) Pretty sure we can learn a lot in 15 mins per person. Let's do a quick study to start with.

I'm assuming the main things we want to learn are 1) What do users thing "Back to site" will do when pressed? 2) How well do users interact with the 'Back to site button?' 3) Do they know where they are when the move between node, admin pages, and then back to previous location? 4) Any issues with placement interfering with their normal tasks?

Any other things we want to learn?

tkoleary’s picture

@lisarex

Maybe do they miss the presence of overlay? Do they even notice it's gone? And do they find it easy to follow the difference between administering the site and browsing the site without it?

Bojhan’s picture

@lisarex Lets discuss on IRC, then we can write down a plan here. I think we need a bunch of users that have used the overlay, and a bunch that haven't at all.

Gábor Hojtsy’s picture

@lisarex: I think we also want to test if people even find that link when asked to go back to the frontend :)

RobLoach’s picture

Forgive my ignorance, but don't most web browsers provide this functionality? I'll read all the comments to get caught up.

[EDIT] So this is to handle sending them back to the original source. Could we also append the "destination" parameter to contextual links?

Gábor Hojtsy’s picture

@Rob: which of those are a frontend page? :) Is it easier to try to cherry-pick that to get back to where you came from vs. hitting a button in the toolbar? Did you try the patch and/or read the goals of it?

tkoleary’s picture

Bojhan: +1

catch’s picture

Category: feature » task
Priority: Normal » Major

@Rob this is more like a breadcrumb than a back button, or the 'continue shopping' link on a shopping cart or similar.

Bumping priority of this - we have two critical issues with the overlay and 2-3 major bugs, so if this ends up replacing it (which would be amazing) it probably ought to be critical. But not bumping priority just yet since I don't want to prejudice the usability testing results.

eigentor’s picture

@lisarex could you link to the UX study announcement from here? I saw it yesterday but could not find it on g.d.o anymore.
Might be very interesting for people stumbling upon this issue.

lisarex’s picture

Sure, here's the node with a link to the study script and signup form
https://groups.drupal.org/node/343023

nod_’s picture

FileSize
4.28 KB

Only changed the name of the behavior, I forgot to rename it when I made the new patch. Rerolled #2088121: Remove Overlay too. I think it'll be even better with the issue that wants to rename "Menu" to "Admin" in the toolbar.

Any non-admin page:
Normal home link

Going in an admin page from a non-admin page:
Admin page back to site link

Going directly to an admin page or opening admin page in new tab:
Admin page normal home link

The overlay module needs to be uninstalled to test this patch.

saltednut’s picture

Title: Add a "Back to..." link so that administrators can return to their most recently visited non-admin page » Add a link so that administrators can return to their most recently visited non-admin page
Category: feature » task
Priority: Normal » Major
Status: Needs review » Reviewed & tested by the community

I've tested this locally and the patch applies cleanly.

Steps:

  1. I modified the standard profile to not install overlay during the install.
  2. Continued testing this using drush uli command and was logged into admin profile page (ie: direct link).
  3. At this point, the link says "Home"
  4. Next step was to click "Home" to visit the front page.
  5. Then open the toolbar to the 'Extend' page.
  6. At this point, the button says "Back to Site" and functions correctly.

I believe this can be committed now.

swentel’s picture

+++ b/core/includes/common.inc
@@ -2214,11 +2214,17 @@ function drupal_add_js($data = NULL, $options = NULL) {
+            'currentPathIsAdmin' =>  $current_path_is_admin,

extreme nitpick ; two spaces after =>. Can be fixed during commit ?

saltednut’s picture

Also worth noting: I have not seen anything break with this patch applied and Overlay installed. With Overlay installed, the button does not ever change from "Home" and the Overlay functions. If one uninstalls Overlay then the functionality described in #95 works as expected.

aspilicious’s picture

Don't we have to wait for the results of the study?

saltednut’s picture

Status: Reviewed & tested by the community » Postponed

@aspilicious Yes, you're correct. Postponing until https://groups.drupal.org/node/343023 is completed.

catch’s picture

Status: Postponed » Needs review

Would rather leave this at CNR - patch might still need updating while usability testing is happening etc.

gmclelland’s picture

Fyi... Processwire at http://processwire.com has a similar ui pattern.

Bojhan’s picture

Small update, we have recruiting running - I am now scheduling participants. Testing will likely start of the weekend, and into next week

tkoleary’s picture

So pending Lisa and Bojhan's results I did some quick testing on a couple of different versions of this around the office. Not as in-depth as what their study does and all done within about 2 hours but it was a good opportunity to test and iterate on design which, added to the other study will hopefully guide us in the right direction.


8 participants:

3 tech savvy users but with some Drupal experience
3 experienced Drupal administrators
2 experienced Drupal developers

Scenario:

Users were shown a prototype of the Drupal admin UI (Caveat, I used ember theme because I had all the components handy in Fireworks) all prototypes started on the front-end home page.

Exploration

You are the content administrator of this World news site. You are looking at the home page of your site and you are in the process of administering content.

Tasks:

  1. OK, now I’d like you to do a series of tasks for me. if you wanted to edit administer the content on this site where would you go?
    Observers: What approach does the participant take?
  2. Now after looking at your list of content you want to go back and check the home page again, how would you do that?
    Observers: What approach does the participant take?
  3. Now that you are on the home page you realize that you want to look at the content on the "world" page of your site, can you do that?
    Observers: This depends on the sample content design and is not relevant to the test so guide the user if needed.
  4. Now after reviewing the content on the "world" page you want to return to the "content" page to make some changes, how would you do that?
    Observers: What approach does the participant take?
  5. Once on the content page you decide to check the comments tab to review comments for spam. How woud you do that?
    Observers: This depends on the Drupal admin tabs (different from core in this prototype) and is not relevant to the test so guide the user if needed.
  6. Now after reviewing the comments you want to go directly back to the "world" page, without clicking first tell me how you would do that?
    Observers: this should surface how the previous navigation experience has set expectations in the users mind.
  7. Now go ahead and click. What just happened? Was it what you expected? Was it clear?
    Observers: how did they respond?

Iterative design/test/design process::

four prototypes were tested in the following order

1 the "back to site" approach proposed by Nod

2 a "last" link with back arrow icon added to the left of the "home" link

Both of these tested about the same. Users basically understood what was happening but there was some confusion over where the home button went and some users still went for the "back" button.

The fourth participant, an experienced Drupal administrator liked the idea of the second prototype but suggested we simply change the word "home" in the link to "site"!!

At that point I had tested four participants, I immediately altered the design and the second prototype to remove the "last" button and change the text of the home button to "site" .

3 the home link text changed to "site"

At this point I re-tested on two new participants and the participant who suggested the "site" link.

These tests went smoothly in every respect, there was no confusion at any point in the scenario except one. One participant less familiar with Drupal said "what if I had completed the task I was doing in the admin and I really did just want to return to the home page, not the last front page?"

My answer to that was to ask "well what would you do then?" to which they answered "click the site's home link". While this is somewhat an edge case it still stuck me as problematic so I went back to the design and altered it again.

This time I took the "site" text and changed it back to "home" but only when the user is in the context of the front-end.

4 the home link text changed to "site", but only in the admin context.

I re-tested the user who had the "completed task" issue, the user who suggested "site" and two new users.

In these last four tests there was absolutely no confusion about how it worked. All four users passed through the test easily and understood what was happening.

Most tellingly all of these users, as well as a few in the previous tests reacted extremely positively to the feature especially at the last question. Some quotes:

Oh cool, it goes right back to where I was

It's like a contextual memory thing

Oh, I love that it becomes "home" when I'm on the front end, that's nice

This will be very helpful

Saves me from having to open another tab

it looks much easier now

Nobody mentioned the overlay or even seemed to realize that it wasn't there.

The final prototype is here: http://invis.io/FZJYO86W

geerlingguy’s picture

Issue summary: View changes
geerlingguy’s picture

Issue summary: View changes
webchick’s picture

So this patch was originally done in DrupalCon Prague (almost 2 months ago).

2 weeks later https://groups.drupal.org/node/343023 was posted to start a usability study. According to https://groups.drupal.org/node/343023#comment-986683 4 tests have been done in that time, and we still need at least 2 more.

Kevin concurrently did a "guerilla usability testing" to try and get us out of deadlock, and there have been zero comments on those results.

What do we need to do here to make progress?

Bojhan’s picture

We did two more sessions and we scheduled tonight to discuss the results. We should be able to write up the summary this week.

It makes me a little sad Kevin did not consult or help out the people who were working hard to get this finalised :(. I appreciate the effort, but the study setup diverges very significantly from the one we setup. I am not sure how comparable or valid the results are.

lisarex’s picture

Findings from this study are posted at https://groups.drupal.org/node/375153.

tl;dr We can conclude from the data that the “Back to site” button works as intended. No major issues were found. It supports the interaction model of being able to quickly go back to the content page.

We're recommending that this patch get into D8. :)

cha0s’s picture

Lol bureaucracy. Super glad to see this happening, thanks all. :)

Bojhan’s picture

We would like to see the recommendations figured out before commit.

I would like to repeat my statement from #73. That nod_ followed a careful process in collaboration with the UX team, and that even with some setbacks we were able to fulfill the gate requirement of usability testing a major UI change before it goes in. I know its sometimes hard to fulfil requirements like this, but it shows our maturity and commitment to UX when we do validate and fine tune before commit.

Although the overlay itself performs quite well in UX testing and solves the disconnect between the admin and changing something in the front end. This solution seems to do achieve this goal as well. Time will tell how well it performs in all the different scenarios our administrators face.

@cha0s Do not confuse bureaucracy with good design practices :)

nod_’s picture

Related issues: +#2088121: Remove Overlay
FileSize
5.16 KB
1.43 KB

Reroll addressing UX testing findings:

Added a "Return to site content" as the title for "Back to site" link.
Fixed the wrong behavior on the block demo admin page.

nod_’s picture

nod_’s picture

This patch doesn't conflict with overlay and both can be active at the same time. No need to wait for #2088121: Remove Overlay to get this one in.

yoroy’s picture

I was not part of the usability testing but I've looked over the raw notes from the observations and saw Lisarex and Bojhan write up their conclusions yesterday. This looks good and we can move forward with it.

The recommendations Bojhan mentions in #110 are these:

  • Proceed with the Back to site button.
  • Amend the ‘hover tip from ‘Home page’ to something that reflects the page user was previously on before, such as “Return to previous content page”, “Return to site content” or “Exit admin screen”
  • Discuss the usefulness of providing a “Home” button when navigating content pages. The usefulness of this button was disputed by 3 of 7 participants.
  • Ensure that if user looks at block demonstration within a theme, then clicks “exit from block demo”, then clicks Back to site, they’ll be directed to the previous non-admin page. Currently they are directed to the Block demo page again. Watch from 8:31-8:57 on http://www.youtube.com/watch?v=sxG_yZfYLgU&feature=youtu.be

We should incorporate fixes for these items in here.

Gábor Hojtsy’s picture

@yoroy: looks like those are fixed in #111.

Bojhan’s picture

No, we should still discuss whether providing a Home button when you are on the front-end - actually adds any value. It adds confusion, because people suddenly see a different button there. And people are not sure why one would use it over the other "home" links, a site with no "home" link is quite rare.

LewisNyman’s picture

I think restricting the toolbar to administrative actions will provide a clearer purpose. If the front-end theme doesn't have a clear back to home link then it is not Drupal's job to fix this. By default Drupal creates a menu item for the home page.

It feels like a hangover, I think we should get rid of it.

It's would be nice to find a solution that doesn't shift all of the toolbar to the right when moving into the admin interface.

Gábor Hojtsy’s picture

I think the idea of the home button in the admin menu is so it is easier to navigate to the home page on constrained devices as well like mobile. The menu sticks to the top of the device while most sites don't have a sticky home button.

nod_’s picture

Home button discussion is valid but out of scope for this issue. Let's proceed here and discuss (and test?) the home button issue separately #2139071: Decide what to do with home link in toolbar.

Wim Leers’s picture

Status: Needs review » Needs work

I wanted to RTBC this, but the code needs some love before that can happen.

  1. +++ b/core/includes/common.inc
    @@ -2214,11 +2214,17 @@ function drupal_add_js($data = NULL, $options = NULL) {
    +          if (!defined('MAINTENANCE_MODE') || MAINTENANCE_MODE !== 'update') {
    +            $current_path_is_admin = path_is_admin($current_path);
    +          }
    

    Why can no path be an admin path while maintenance mode is enabled? Could use a comment to explain this condition.

  2. +++ b/core/modules/block/lib/Drupal/block/Controller/BlockController.php
    @@ -30,6 +30,13 @@ public function demo($theme) {
    +        'js' => array(
    +          array(
    +            // Let JavaScript know this really is an admin page.
    +            'data' => array('currentPathIsAdmin' => TRUE),
    +            'type' => 'setting',
    +          )
    +        ),
    

    The real question is: why doesn't path_is_admin() mark this as an admin path?

    You can use hook_admin_paths() to mark this path as administrative, then there is no more need for this hack.

  3. +++ b/core/modules/toolbar/js/escapeAdmin.js
    @@ -0,0 +1,36 @@
    +(function ($, Drupal, drupalSettings) {
    

    File docblock missing.

  4. +++ b/core/modules/toolbar/js/escapeAdmin.js
    @@ -0,0 +1,36 @@
    +  "use strict";
    

    This code, and further code is indented because it's in a closure; this is not the case for other JS in Drupal.

  5. +++ b/core/modules/toolbar/js/escapeAdmin.js
    @@ -0,0 +1,36 @@
    +  var escapeAdminPage = sessionStorage.getItem('escapeAdminPage');
    

    Shouldn't sessionStorage be injected in the closure?

  6. +++ b/core/modules/toolbar/js/escapeAdmin.js
    @@ -0,0 +1,36 @@
    +  if (!drupalSettings.currentPathIsAdmin) {
    

    Should have a comment.

  7. +++ b/core/modules/toolbar/js/escapeAdmin.js
    @@ -0,0 +1,36 @@
    +  Drupal.behaviors.escapeAdmin = {
    

    Docblock missing.

  8. +++ b/core/modules/toolbar/js/escapeAdmin.js
    @@ -0,0 +1,36 @@
    +      var $toolbarEscape = $('[data-toolbar-escape]').once('escapeAdmin');
    

    Is all this "escape" stuff really the appropriate name?

    If it is, then I think it should *always* be called "escape to admin" or in classes: "escape-admin", not "escape". So s/data-toolbar-escape/data-toolbar-escape-admin/, too. Just "escape" in a "toolbar" context does not mean anything.

  9. +++ b/core/modules/toolbar/js/escapeAdmin.js
    @@ -0,0 +1,36 @@
    +              'href': Drupal.url(escapeAdminPage),
    

    s/Page/Path/?

  10. +++ b/core/modules/toolbar/toolbar.module
    @@ -609,6 +615,18 @@ function toolbar_library_info() {
    +    'title' => 'Provides a button to escape the admin.',
    

    s/the admin/the administration area/

Bojhan’s picture

Is there any reason we can't just fix it here? I love followups that never get solved. But the scope of this patch is extremely limited, and we basically identified only 1 discussion point from all of the testing. Its completely fair asking that to be resolved before commit.

nod_’s picture

FileSize
5.87 KB
5.92 KB

Addressed the doc concerns. I think. I have two changes not in the interdiff (interdiffed too early, anyway).

Didn't add sessionStorage to the closure as that wouldn't be a dependency if we were using AMD (which is the point of those new declarations in file-closures).

nod_’s picture

Status: Needs work » Needs review
nod_’s picture

FileSize
6.42 KB

hidding home button on frontend and closing other issue.

Wim Leers’s picture

  1. +++ b/core/modules/toolbar/js/escapeAdmin.js
    @@ -1,36 +1,49 @@
    + * Replaces the Home link with Back to site link.
    ...
    + * Back to site link points to the last non-administrative page the user visited
    

    s/Home/"Home"/
    s/Back to site/"Back to site"/

  2. +++ b/core/modules/toolbar/js/escapeAdmin.js
    @@ -1,36 +1,49 @@
    + * withing the same browser tab.
    

    s/withing/within/

  3. +++ b/core/modules/toolbar/css/toolbar.module.css
    @@ -38,6 +38,9 @@
    +.toolbar .toolbar-bar .toolbar-tab.hidden {
    +  display: none;
    +}
    

    Why is this necessary? Shouldn't system.module.css's CSS already handle .hidden? Maybe not due to selector specificity? If so, a comment explaining that would be appropriate.

nod_’s picture

Jeff Burnz’s picture

My apologies if this has been covered before but this has some rather unexpected behaviour.

For example navigate Home -> Content -> Comments -> Edit a comment, then save it (which redirects you back to the Comments page). Now the "Back to site" button links to the comment edit form I just finished with.

This is not what I expected - I thought I would go back to the Home page.

nod_’s picture

Ok this is the same issue as the block demo page.

We could add the currentPathIsAdmin to it as well but then the question is where else we're displaying the frontend theme for administration pages?

jessebeach’s picture

Let's just blacklist those two cases for now and deal with others that we find later. I can't think of any others right now.

nod_’s picture

Might be more complicated than that unfortunately #2139341: path_is_admin() is abused by "use_admin_theme"

David_Rothstein’s picture

+            // The block demonstration page is not marked as an administrative
+            // page by path_is_admin() function in order to use the frontend
+            // theme.

This isn't really correct; see https://api.drupal.org/api/drupal/core!modules!block!block.module/function/block_admin_paths/8 for the actual reason (basically, to keep it out of the Overlay). A simple fix might be just to get rid of block_admin_paths() and move that hook implementation to the Overlay module. That way at least the problem only occurs when this feature is being used in conjunction with Overlay (presumably not too common anyway since the two features do the same thing). The real fix for it might eventually be #1220234: Introduce path_get_tagged_paths, and allow the overlay to be used on non-admin pages.

For the comment edit form, any reason that shouldn't just be declared as an admin path directly?

Will comment on #2139341: path_is_admin() is abused by "use_admin_theme" directly.

nod_’s picture

FileSize
832 bytes
6.61 KB

Ok I've been thinking about it.

If people uncheck the "Use the administration theme when editing or creating content" to display node edit forms within the frontend theme, they don't consider those as "administrative" so if they edit a node, and go to the module page, the "back to site" link would send them back to the node edit form. It makes sense after all.

The block demo page is clearly different so it's valid to special-case it.
On the other hand the comment form should probably be an administrative path (by default at least). So that shouldn't be special-cased here.

One problem would be if an editor wants to edit a bunch of nodes from the admin/content page while the node edit form isn't displayed in the admin theme. It means that the back to site button links back to the latest node form he edited, probably not what he expects.

All in all it makes sense as-is. Question is, it satisfactory or not?

The attached patch doesn't save the last page if there is a "destination" parameter in the URL. That takes care of editor going back and forth between admin/content page and node edition. Solves the use case in #127 too.

Status: Needs review » Needs work

The last submitted patch, 132: core-overlayslayer-ux-approved-787896-132.patch, failed testing.

nod_’s picture

Status: Needs work » Needs review
jessebeach’s picture

+++ b/core/modules/toolbar/js/escapeAdmin.js
@@ -0,0 +1,53 @@
+
+/**
+ * Replaces the "Home" link with "Back to site" link.
+ *
+ * Back to site link points to the last non-administrative page the user visited
+ * within the same browser tab.
+ */
+Drupal.behaviors.escapeAdmin = {

Have we agreed to remove the Home tab? Because if so, I'll remove it for real in Toolbar and we can insert the 'Back to site' tab with a theme function rather than hiding it in the DOM until we need it.

Bojhan’s picture

I have looked at nod_ his patch and issue, but I find it difficult to find the usecase where it actually breaks down.

nod_’s picture

FileSize
6.48 KB
1.68 KB

Jesse is right, no point in keeping the home link around. So I just created the back to site link from PHP, it removes a few lines of JS so it's always good. I don't want to have to deal with any HTML on the JS side (for this anyway). It's just simpler this way, there is no impact on users and the JS will be faster. Everybody wins.

I removed the escape key event handler. It was more for the fun of it than anything. Nobody beside me ever mentioned it so It's time for it to go (this change is missing from interdiff, but it's in the patch).

I think we're all set now: Bojhan can't see the issue I had, #127 is solved in a way that's not comment-specific and it's all still very simple.

nod_’s picture

FileSize
972 bytes
7.21 KB

Removing some toolbar CSS related to non-existing home link.

nod_’s picture

FileSize
7.24 KB
420 bytes

I had one dependency missing in the library declaration.

I think we're all set now. Sorry for the patch spaming.

Status: Needs review » Needs work

The last submitted patch, 137: core-overlayslayer-ux-approved-787896-137.patch, failed testing.

nod_’s picture

Issue summary: View changes
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 138: core-overlayslayer-ux-approved-787896-138.patch, failed testing.

nod_’s picture

Status: Needs work » Needs review

Drunk testbot, last patch is green.

webchick’s picture

Can we get some updated screenshots for this issue? The next opportunity I have to talk with Dries about this is on Tuesday. I'll try and give it a spin myself before then.

nod_’s picture

thedavidmeister’s picture

those screen grabs look so much more elegant than overlay, yay!

jp.stacey’s picture

Just applied the patch in #139 and got a hunk offset, which gives me an excuse to re-roll @nod_'s patch and attach for testing. Let's see if drunk testbot is drunk :)

The code looks good but I'd appreciate another eye on it. There's a use of the identity operator for string comparison, which I wasn't sure about initially but seems to be used frequently in D8 core. If that's been adopted as standard, could it be added to the coding standards page?

lisarex’s picture

I gave the patches in #139 and #147 a spin the today and all is looking great. (I haven't looked at the code though). All of our recommendations in the UX research findings have been addressed https://groups.drupal.org/node/375153

ksenzee’s picture

Status: Needs review » Reviewed & tested by the community

Patch looks good to me. I say we kill it dead.

Bojhan’s picture

@webchick asked me to provide more rationale why we are removing the "Home" button. By changing the purpose of the top left corner, we are introducing a UX issue. We can distinguish three reasons:

1) The proposition of the "home" button is now unclear. Given that its only shown in the context of "often" many other home buttons like the logo/home link in navigation. People percieve that the button, as it is currently shown has less value.

2) People are occasionally (3/7) thrown off by the changing purpose of the top left item. Some cases its "back to site" other cases its "home". It doesn't feel very consistent, and it takes them a bit to understand why which one is shown.

3) The home button on mobile has seen its share of issues, by removing it we bring the most important button "menu" more closer to a easy to hit area. With that making it better for mobile useage.

We did a fair bit of searching to find usability research that supports the removal of a home button. There is none, that we found that covers the rare usage that we have of using the home link in "application level navigation" vs. on the "website level navigation". Although experts often advise to have a "home link" (e.g. Steve Krug), there is no documentation that argues for or against having an application level home link on the front-end.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for all the work on this issue, and specifically the UX testing. Committed to 8.x.

Goodbye overlay, and thanks for all the fish.

LewisNyman’s picture

The home button on mobile has seen its share of issues, by removing it we bring the most important button "menu" more closer to a easy to hit area. With that making it better for mobile useage.

Actually we've just moved it into the hardest to hit area. I'm tended to pull #1800584: Allow the toolbar tray to be dismissed with a swipe event back into 8.x

nod_’s picture

left handed are people too.

happy to work on the other issue if needed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Dave Reid’s picture

FYI I made a module to alter the behavior of D7's core toolbar or the contrib navbar module to do this: https://drupal.org/project/escape_admin

rooby’s picture

@Dave Ried:
Champion! :)