Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett created an issue. See original summary.

larowlan’s picture

Status: Postponed » Active
MaskOta’s picture

Don't these mockups need to be updated first too? Unless i am misunderstanding something you are not managing the layout on the on the display of the node anymore.

Also we have links to add blocks in all regions which are not shown on the current mockup.
The button to remove the section is missing

EclipseGc’s picture

This particular mockup is indeed not completely accurate. Multiple blocks can be added to any region, and that doesn't seem to be reflected here.

Eclipse

tim.plunkett’s picture

Issue summary: View changes

Correct. Updated the IS with the link to the latest mocks

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tim.plunkett’s picture

Component: layout.module » layout_builder.module
tim.plunkett’s picture

Assigned: Unassigned » DyanneNova
Priority: Normal » Major
Issue tags: +sprint

Tagging as this is currently on our shortlist.

DyanneNova’s picture

Here's a small patch to update the colors and add the plus button. The markup needs to be changed to move the delete button, and to move the blue background on add block to only the empty area. I'm working on the draggable affordance next, but this small patch gets us one step closer to the mockups.

This patch gets us to this state:
Screenshot of changes to Layout Builder

DyanneNova’s picture

This patch adds a fix for the first issue in #2940212: [meta] Miscellaneous UI issues for the Layout Builder module, by adding a background and outline to blocks when dragging, along with adding visible drop zones for the dragged content.

screen gif of draggable styles

tim.plunkett’s picture

  1. +++ b/core/modules/layout_builder/css/layout-builder.css
    @@ -7,19 +8,50 @@
    +.ui-sortable-helper {
    ...
    +.ui-sortable-placeholder {
    

    Just for paranoia's sake, can these be prefixed with .layout-section or something more top-level to prevent it from changing something else on the page? (I really hope there'd be nothing else, but you never know)

  2. +++ b/core/modules/layout_builder/css/layout-builder.css
    @@ -7,19 +8,50 @@
    +  visibility: visible !important;
    

    Usually we avoid !important, but in the case of jQuery UI there's not too much choice.

EclipseGc’s picture

Can we get a different color for either the droppable area or the add block links? I wouldn't mind the add block links to be more "buttony" in their style and keep the blue drop zone styling as is. Thoughts?

Eclipse

DyanneNova’s picture

Here's a patch with prefixed ui classes, and I've set a placeholder class in the js, so that we don't need to use important now. Thanks for pointing that out and getting me thinking about that over again!

DyanneNova’s picture

I've changed the drop zone to a yellow to distinguish it from the add block regions. I think you're right, it makes sense to distinguish movement vs adding.

screengrab of yellow drop zone

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

This looks great and is a huge step up from what we have, and can be iterated upon later.

tim.plunkett’s picture

... except the change needs to be in the ES6 file, and transpiled to the regular JS file.
Fixing, and leaving at RTBC

EclipseGc’s picture

++ing the rtbc on this. Seems like a really good step.

Eclipse

lauriii’s picture

Status: Reviewed & tested by the community » Needs work

Sorry that I haven't been able to give you feedback earlier. The UI improvements look great. There's one improvement that I wish we could do for the markup and the CSS:

+++ b/core/modules/layout_builder/css/layout-builder.css
@@ -7,19 +8,49 @@
+.add-section a, .add-block a {
...
+.add-section a:hover, .add-section a:active, .add-section a:focus,
+.add-block a:hover, .add-block a:active, .add-block a:focus {

The link should have a class so we don't have to use CSS the combinators. Something like .add-section__link would work.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
1.99 KB
3.86 KB

Okay

DyanneNova’s picture

Status: Needs review » Reviewed & tested by the community

This looks good to me.

pifagor’s picture

Looks good for me.

lauriii’s picture

Status: Reviewed & tested by the community » Needs work

There's still following stylelint errors that are blocking the commit:

modules/layout_builder/css/layout-builder.css
 11:19  ✖  Expected newline after ","        selector-list-comma-newline-after
 18:25  ✖  Expected newline after ","        selector-list-comma-newline-after
 18:52  ✖  Expected newline after ","        selector-list-comma-newline-after
 19:23  ✖  Expected newline after ","        selector-list-comma-newline-after
 19:48  ✖  Expected newline after ","        selector-list-comma-newline-after
 21:10  ✖  Expected "#000000" to be "#000"   color-hex-length
 34:21  ✖  Expected "#ffffdd" to be "#ffd"   color-hex-length
joelpittet’s picture

Status: Needs work » Needs review
FileSize
3.86 KB
1.16 KB

Here's the stylelint fixes.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

  • lauriii committed 377dddf on 8.6.x
    Issue #2916877 by DyanneNova, tim.plunkett, joelpittet: Update Layout...

  • lauriii committed ad2a831 on 8.5.x
    Issue #2916877 by DyanneNova, tim.plunkett, joelpittet: Update Layout...
lauriii’s picture

Assigned: DyanneNova » Unassigned
Status: Reviewed & tested by the community » Fixed

Thank you. The changes look good now.

Committed 377dddf and pushed to 8.6.x and cherry picked ad2a831 and pushed to 8.5.x. Thanks!

Ps. this patch was committed 41000ft. on top of East China Sea on the way to DrupalCon.

Status: Fixed » Closed (fixed)

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