Problem/Motivation

The Outside In prototype introduced in #2753941: [Experimental] Create Outside In module MVP to provide block configuration in Off-Canvas tray and expand site edit mode makes it very easy for a user to understand which elements on the page are configurable and to select them in an intuitive way. Once the element is selected, a sidebar for it opens.

However, once the sidebar opens, there is no longer any visual indication of what sidebar element is being configured.

This can become especially confusing for some of the more unintuitive/magical blocks, for example the title block. See #2782891: The Page Title block's title behaves in a confusing way with Settings Tray and the Help block incorrectly has Settings Tray styling. That problem is exacerbated by not even being able to see that that is what I clicked on.

Proposed resolution

Come up with a design to visually highlight the region on the page that is being configured.

Remaining tasks

Needs design, usability review, implementation, etc.

User interface changes

New visual indication of the element being configured (TBD).

API changes

TBD; most likely none.

Data model changes

Probably N/A.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm created an issue. See original summary.

xjm’s picture

Issue summary: View changes
xjm’s picture

Version: 8.3.x-dev » 8.2.x-dev
xjm’s picture

This feels major actually.

xjm’s picture

Priority: Normal » Major
tkoleary’s picture

This is a case of a feature of the design that has not yet been implemented.

The prototype shows that the highlight effect that a block gets on hover persists when the block is selected and the tray is open.

xjm’s picture

Thanks Kevin. Sounds like this is a "Must-have" part of implementing the full design.

webchick’s picture

Issue tags: +sprint
tedbow’s picture

Component: contextual.module » outside_in.module
tedbow’s picture

Ok here is patch that passing the id of the active element to the dialog and then adds a class to that element on dialog open so it can be highlighted.

I include some initial css to highlight the element.

The patch includes the changes from #2786459-59: "Offcanvas" tray should be using the existing dialog system

Also a patch with only relevant changes.

tedbow’s picture

Forgot patch with only relevant changes.

tedbow’s picture

Status: Active » Needs review
tedbow’s picture

#2786459: "Offcanvas" tray should be using the existing dialog system landed!

re-rolled with just these changes

@tkoleary this should work but the css should be improved so that active element stands out.

GrandmaGlassesRopeMan’s picture

+++ b/core/modules/outside_in/js/outside_in.js
@@ -142,9 +142,29 @@
+        $('body').find('*').removeClass('outside-in-active-editable');

Instead of fetching all elements in the body, just grab the ones that have the class and then remove it.

  $('body .outside-in-active-editable').removeClass('outside-in-active-editable')
tedbow’s picture

@drpal nice catch. I have fixed that

GrandmaGlassesRopeMan’s picture

Status: Needs review » Reviewed & tested by the community

@tedbow

Looks good. I'm happy with the most recent set of changes.

webchick’s picture

Issue tags: +Needs screenshots

Hm. I applied the patch to simplytest.me, but can't tell what I'm looking for here. Got screenshot?

tedbow’s picture

Issue summary: View changes
FileSize
138.55 KB

@webchick here is screenshot

Notice that the breadcrumb block which is the one that is being updating in the tray has red-dotted border whereas all the other block have a gray dotted-border.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

Yeeah, that's way too subtle. :( Also doesn't really work on e.g. the site branding block, because the contrast isn't clear enough.

Let's go with Kevin's suggestion in #6 to persist the highlight styling on the active element.

tedbow’s picture

Status: Needs work » Needs review
FileSize
2.23 KB
1.06 KB
105.92 KB

@webchick ok.

This patch just adds the active element selector to the hover selector so they are same. much simpler and better ux I think.

tedbow’s picture

Re-rolled the patch from #20. It no longer applied.

Also fixed 2 JS lines that didn't end with ";". No other changes

GrandmaGlassesRopeMan’s picture

@tedbow @webchick

I've tested this locally and while it is working, its also not obvious, at least initially, what's happening. The active effect and the hover effect appear to be nearly identical. Unless you immediately move your mouse away, the active could easily be mistaken for the hover styling. In addition to the darkened background, perhaps the red dashed outline as well, although that might be too garish.

tkoleary’s picture

@drpal

perhaps the red dashed outline as well, although that might be too garish.

The prototype just maintained the same hover effect as the active state once the tray was open. I don't think we need the red, esp. since red indicates warning or error.

tkoleary’s picture

Status: Needs review » Reviewed & tested by the community

Looks right now.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 21: 2782885-21.patch, failed testing.

tedbow’s picture

Status: Needs work » Reviewed & tested by the community

Looks like unrelated test failed but then ran again and everything passed.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

#2785589: Fix js and jsdoc of outside-in module got reverted which means this needs a reroll.

rakesh.gectcr’s picture

Status: Needs work » Needs review
FileSize
2.17 KB

I have re-rolled the patch against the 8.2.x

nod_’s picture

Issue tags: +JavaScript
tkoleary’s picture

Status: Needs review » Reviewed & tested by the community

Tested in simplytest.me. Works as expected.

Great work!

webchick’s picture

Status: Reviewed & tested by the community » Needs work

This no longer applies after #2781577: Properly style outside-in off canvas tray. Anyone up for a quick re-roll?

tedbow’s picture

Status: Needs work » Needs review
FileSize
2 KB

@webchick ok here it is. Just a re-roll

webchick’s picture

Status: Needs review » Reviewed & tested by the community

Moving back to RTBC, since there are no substantial changes.

webchick’s picture

Committed and pushed to 8.2.x and 8.3.x. Thanks!

  • webchick committed 58f67f6 on 8.3.x
    Issue #2782885 by tedbow, rakesh.gectcr, xjm, webchick, tkoleary, drpal...

  • webchick committed 2e6a847 on 8.2.x
    Issue #2782885 by tedbow, rakesh.gectcr, xjm, webchick, tkoleary, drpal...
webchick’s picture

Status: Reviewed & tested by the community » Fixed
Manuel Garcia’s picture

Issue tags: -Needs reroll

Status: Fixed » Closed (fixed)

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

Gábor Hojtsy’s picture

Issue tags: -sprint
tedbow’s picture

Component: outside_in.module » settings_tray.module

Changing to new settings_tray.module component. @drpal thanks for script help! :)