Inconsistent use of styles on Flexible Template Builder UI

[jenlampton]

Description of the bug

The UI for building a Flexible Template has some unusual CSS patterns that are not consistent with the rest of core. This page should have similar design patterns to the Manage Blocks page. That said, the elements we are moving around are not blocks, so they should not match the pattern for blocks on the manage block page to avoid confusion.

Some individual problems are noted below, but this page could probably use a a design review / refresh as a whole.

• Strange spacing between border and interior
• use of blue dotted line (unique to only this page)
• box shadow too large (should be box-shadow: 2px 2px 0 0 #d0d0d0; for consistency)
• combination of box-shadow and dotted line looks strange

[stpaultim]

I took a stab at making some improvements here. Sure, we could always use a designer input, but is this good enough (with feedback) to move forward until we get a designer. (Actually, I did get input on most of the changes I made today from a designer at Twin Cities Drupal January sprint).

Before:

After (CURRENT PR):

For reference, this is the current block layout page (manage blocks on layout):

I welcome thoughts and ideas for improvement.

Atten: @jenlampton

[yorkshire-pudding]

@stpaultim - it is a definite improvement. I don't have any suggestions for improvement and I would be happy with this.

[stpaultim]

@yorkshire-pudding I put this on the agenda for the design/ux meeting next week.

I'm open to suggestions for how to make this revision better OR we could work incrementally and make more changes later.

[yorkshire-pudding]

@yorkshire-pudding I put this on the agenda for the design/ux meeting next week.

I'm open to suggestions for how to make this revision better OR we could work incrementally and make more changes later.

Unless there are concrete suggestions, I'm a strong fan of incremental improvement

[klonos]

Some feedback from today's dev meeting:

1. looks good and is definitely an improvement 👍🏼
2. the cursor on-hover over the rows should be a "vertical move" cursor (up/down) - while the current cursor is a "move cursor" (all directions, which is wrong for the flexible layouts UI, as the rows can only be moved up/down), and the PR changes it to a default "select text" cursor (which is a regression).
3. Related to the above, the "move" icon (the one to the left of the row name) will also need to change (but it seems that we will need to introduce a new icon for that). Related: Replace the draggable.png icon with a modern one, and add up/down-only and left/right-only variants. #5152
4. some attention in the left/right/up margins of the row headers (they should be removed, so that the row "attaches" to the border of the container).
5. The font size of the row header text is smaller than the respective used in the blocks layout - needs to be adjusted.

(I hope I captured everything)

[stpaultim]

PR is about to get updated:

I changed the cursor back to the one we were using before (fixed accidental regression). I did not do an up/down cursor, because I did not see one in standard CSS and it seems we have not yet implemented #5152

In regards to @klonos item 5) My inspection shows same size fonts for header on Blocks Layouts and on Flexible Layouts Template configuration. 1.1em or 17.6px

[stpaultim]

Incorporated feedback from @jenlampton . Thanks so much for taking a look. I appreciate it.

[klonos]

@stpaultim #5152 is in order to change the drag icon (the one to the left of the row titles) to a .svg provided by the Phosphor icon set in core (point 3 in my previous comment in this thread), whereas what needs to be changed to up/down is the standard mouse cursor CSS property to be cursor: row-resize; (which is point 2 in my previous comment). See https://www.w3schools.com/cssref/pr_class_cursor.php and https://www.w3schools.com/cssref/playit.php?filename=playcss_cursor&preval=row-resize specifically.

One thing that has been mentioned before is the lack of sufficient contrast with the various shades of gray and white we use, but we have #1632 for that. What we could do here though is to add some border to the regions. If we go with the same dashed border as in the "Manage blocks" UI (border: 1px dashed #CCC; instead of the current border: 1px solid white;), then it would suffice and be consistent I believe.

Other than those issues, I think that this good to go. Thanks @stpaultim 🙏🏼

[klonos]

If we go with the same dashed border as in the "Manage blocks" UI ...

...actually, simply removing the following fixes that (I've added a comment in the PR):

.l-flexible-row .layout-editor-region {
  border: 1px solid white;
}

[stpaultim]

standard mouse cursor CSS property to be cursor: row-resize;

@klonos I looked for a standard cursor that did this and did not find this one on my own. Thanks for pointing this out. Change made, along with your other suggestions.

Does anyone else have any thoughts on this PR?

[jenlampton]

@klonos @stpaultim As soon as we get rid of the dotted lines on the regions, I think this is RTBC!

The dotted lines mean "you can drop things here" and since we are dragging rows on this page (and not blocks) and since rows cannot be dropped into regions, the dots need to go. (This was discussed very early on, I think, you may have missed that part of the discssion @klonos.)

[jenlampton]

We could be a little more consistent with the manage blocks interface if we made the main background gray, and also provided a drop-zone for rows with a white background and gray dashed border.

Manage blocks page with drop zone:

Flexible layout page with drop-zone:

I'll make a PR against your PR @stpaultim

[jenlampton]

@stpaultim I'm not sure I like this any better, so I provided an alternate PR (instead of one against yours) where people can see what the page would look like with a drop-zone for rows: backdrop/backdrop#4738

Here's the page with an additional drop-zone for rows added (more consistent with manage blocks)

I think I prefer the current PR but without the drop-zone for blocks:

@klonos would love your opinion here too :)

[stpaultim]

In terms of consistency, I think that there is enough of a difference between the manage blocks and flexible layouts page that consistency is at least somewhat in the eye of the beholder. It's not clear to me that either approach is objectively more consitent?

Having said that, I think I would vote for the second one. BUT, I'd be fine with either one.

@klonos asked me to put the dotted line around the regions to be more consistent with the manage blocks page. Then you removed it. I'm not sure why or if that was intentional. On the manage layouts page, regions have a dotted/dashed border. I had added that to my PR for consistency with flexible layout page, but your PR against my PR removed it again. Was that intentional?

@jenlampton

[stpaultim]

At @jenlampton My last version had this dashed line (actually, the dashed line is inherited from existing css, I had overridden it with a solid line, but I removed the override in my last draft at the suggestion from @klonos).

The manage blocks page puts this around regions as well.

Your PR against my PR overrides that dashed line again and makes it solid again.

[klonos]

The dotted lines mean "you can drop things here" and since we are dragging rows on this page (and not blocks) and since rows cannot be dropped into regions, the dots need to go. (This was discussed very early on, I think, you may have missed that part of the discssion @klonos.)

Gotcha 👍🏼 ...makes sense. I didn't know that that was the convention, and yes, I missed the discussion about it. Sorry.

Here's the page with an additional drop-zone for rows added ... @klonos would love your opinion here too :)

Yeah, I don't think that dropzones in the template UI makes sense in the way that region dropzones make sense in the blocks UI.

@klonos asked me to put the dotted line around the regions to be more consistent with the manage blocks page. Then you removed it. I'm not sure why or if that was intentional. ...

Apologies for the confusion @stpaultim ...I did miss the previous discussions, and what @jenlampton is describing makes sense. My thought process when I made the suggestion I made was more like this: I noticed that there is some rather serious contrast issue in the tones of white/gray we are using around the regions and their row container -> I then opened another tab with the blocks UI in order to compare and see why this issue was not as prominent to my eyes before -> then saw that the regions in the block UI have the dotted border, which made things a bit better -> then tested the same in my local with the template UI and it seemed like an improvement. Now that I know that there is a mental association with the dotted border as a dropzone, I realize that what Jen is saying makes sense. ...so if I'm not asking too much, can we then please try changing the dotted line to a solid one, but with the same gray color (#ccc I believe)? (instead of the solid white like you had it in your original PR, which made no difference anyway)

[klonos]

...can we then please try changing the dotted line to a solid one ...

Never mind, I see that that's been done in the latest commits. Re-reviewing 👀

[stpaultim]

So, I think my version of the PR has all the requested changes and the agreed upon version of the drop zone (or lack thereof).

@klonos and @jenlampton - What next?

[klonos]

@stpaultim I've filed a PR against your PR: stpaultim/backdrop#7

Before you merge it, I'd like @jenlampton's and other people's 👀 on it please. See comments and screenshots in the PR summary.

[stpaultim]

@klonos I agree that the contrast is not great pretty bad. I tried to keep my focus on consistency between the two pages and was reluctant to make this kind of improvement, for fear that scope creep (keeping two pages consistent, while making improvements) would bog down the issue.

Having said that. Your scope creep is ONLY 1-2 lines of css and only effects one other file. I think this increase in scope is fine (and preserves consistency) and I do think it makes the two pages easier to read or look at. So, I'm in favor of your change, if others are ok with it as well and as long as it does not open the door to a bunch of other new "improvement" requests.

Happy to file a follow-up issue to take the redesign even further at a later date.

[klonos]

Adding the screenshots for the PR here, so that people can nay or yay in order to move this issue on:

...adds consistent 1px solid #cccccc; in various places - see before(Tim's PR)/after(this PR) side-by-side screenshots:

• template UI:
  
• blocks UI: