Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reveal block boundaries on hover in the Site Editor #27271

Merged
merged 16 commits into from
Dec 1, 2020

Conversation

jameskoster
Copy link
Contributor

@jameskoster jameskoster commented Nov 25, 2020

This PR roughly implements parts of a prototype I worked on for #27239, to see how it feels more holistically.

boundaries

Block boundaries are exposed on hover making it easier to understand what will be selected upon click. This is important in the Site Editor particularly, because block nesting features prominently in that context.

A different color is applied to template parts, to indicate that they are different to regular blocks.

This feels like a net positive change to me, although I would say that the is-hovered class seems to be somewhat brittle. Moving the cursor around quickly can trip it up. It doesn't occur all that often, but if there's any way to improve performance there it would be worth exploring.

To test:

  • Load up a template in the Site Editor
  • Apply hover, select, focus, select+focus states to various blocks and ensure the following:
  1. Un-selected, hovered blocks display a 1px blue outline
  2. Selected + focussed blocks (IE on-click) display a 1.5px blue outline
  3. Selected un-focussed blocks display a 1px black outline

@jameskoster jameskoster added Needs Design Feedback Needs general design feedback. [Feature] Full Site Editing [Type] Discussion For issues that are high-level and not yet ready to implement. labels Nov 25, 2020
@jameskoster jameskoster linked an issue Nov 25, 2020 that may be closed by this pull request
@jameskoster jameskoster marked this pull request as draft November 25, 2020 12:31
@github-actions
Copy link

github-actions bot commented Nov 25, 2020

Size Change: +38 B (0%)

Total Size: 1.19 MB

Filename Size Change
build/block-editor/style-rtl.css 11.2 kB -55 B (0%)
build/block-editor/style.css 11.2 kB -52 B (0%)
build/block-library/editor-rtl.css 8.86 kB -84 B (0%)
build/block-library/editor.css 8.87 kB -83 B (0%)
build/block-library/style.css 8.27 kB +1 B
build/edit-site/style-rtl.css 4.06 kB +156 B (3%)
build/edit-site/style.css 4.06 kB +155 B (3%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.8 kB 0 B
build/api-fetch/index.js 3.42 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 665 B 0 B
build/block-directory/index.js 8.72 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 128 kB 0 B
build/block-library/index.js 148 kB 0 B
build/block-library/style-rtl.css 8.27 kB 0 B
build/block-library/theme-rtl.css 789 B 0 B
build/block-library/theme.css 790 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 172 kB 0 B
build/components/style-rtl.css 15.3 kB 0 B
build/components/style.css 15.3 kB 0 B
build/compose/index.js 9.95 kB 0 B
build/core-data/index.js 14.8 kB 0 B
build/data-controls/index.js 827 B 0 B
build/data/index.js 8.98 kB 0 B
build/date/index.js 11.2 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.95 kB 0 B
build/edit-navigation/index.js 11.1 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.42 kB 0 B
build/edit-post/style.css 6.4 kB 0 B
build/edit-site/index.js 24.1 kB 0 B
build/edit-widgets/index.js 26.3 kB 0 B
build/edit-widgets/style-rtl.css 3.13 kB 0 B
build/edit-widgets/style.css 3.13 kB 0 B
build/editor/editor-styles-rtl.css 476 B 0 B
build/editor/editor-styles.css 478 B 0 B
build/editor/index.js 43.3 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.85 kB 0 B
build/element/index.js 4.62 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.86 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.27 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 698 B 0 B
build/keyboard-shortcuts/index.js 2.54 kB 0 B
build/keycodes/index.js 1.93 kB 0 B
build/list-reusable-blocks/index.js 3.1 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.31 kB 0 B
build/notices/index.js 1.82 kB 0 B
build/nux/index.js 3.42 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.43 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/reusable-blocks/index.js 2.92 kB 0 B
build/rich-text/index.js 13.4 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 2.84 kB 0 B
build/viewport/index.js 1.86 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@@ -208,7 +208,7 @@

// Active entity spotlight.
&.has-active-entity:not(.is-focus-mode) {
opacity: 0.5;
//opacity: 0.5;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need to remove this when this PR goes out of draft state.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably 😅

@jasmussen
Copy link
Contributor

Thank you for this PR. As will be evident from the GIF below, my site editor is currently a little messy, so it's not totally accurate. But hopefully a few aspects can serve as legitimate feedback:

test

First off, the rendering issues with the 1.5px border are present here. I've a bit of tasks on my plate today, but I'm happy to try and dive in tomorrow and see if I can help fix it.

Secondly, and as noted in #27239, I think we should avoid different colors for template part boundaries. Right now we have black and blue colors (in the default color scheme), and those are sufficiently challenged on black and blue backgrounds, that I think we need a smarter system for rendering colors on arbitrary backgrounds before we introduce additional ones.

border

In this one, there's both some weirdness with the hover style (there's a dead zone). But also the fact that it looks like the border of the placeholder box grows thicker. This is probably a coincidence, because the hover is 1px outset from the border of the placeholder.

I wonder if we should have a spot colored 1px hover style, instead of black? This would make blue the color for all things selected/focused:

  • Hovered: 1px blue
  • Selected and focused: 1.5px blue border
  • Not part of this PR, but could be explored in the future: selected but not focused: 1px blue border.

What do you think?

@jameskoster
Copy link
Contributor Author

Updated to focus on hover interactions only for now (removed template part colors).

I think the selected / focused / hovered outlines are behaving as expected, but I am not 100% familiar with the interaction there so more eyes would be helpful. One question I have there is: which treatment takes priority? IE if I hover a selected item, which outline style should prevail, 1px (hovered) or 1.5px outline (selected)?

@jasmussen
Copy link
Contributor

One question I have there is: which treatment takes priority? IE if I hover a selected item, which outline style should prevail, 1px (hovered) or 1.5px outline (selected)?

There are no rules to follow here, so it's what we choose as best. Would it be useful for a focused block to have a hover effect at all?

Note, I figured out why the focus outline is weirdly cropped like so:

Screenshot 2020-11-26 at 10 41 18

If you add outline: 2px solid transparent; to that focus style, it looks right:

Screenshot 2020-11-26 at 10 40 31

The benefit is that this focus style will also be visible in windows 10 high contrast mode.

@jameskoster
Copy link
Contributor Author

Updated so that the selected style persists over the hover style. Since the hover style intends to outline what will be selected on click, it's probably not adding much value when the block is already selected.

@jasmussen
Copy link
Contributor

Current status:

status

This is looking like a good small step that I think is worth trying. And most of the issues appear to have been fixed.

It looks to me like some of the blue borders are used incorrectly in the site editor — that's the case also in the master branch so not the fault of this PR — but they linger even if focus moves elsewhere. The thick blue border is focus only, so we should look at refactoring it to actually be attached to :focus. We can absolutely explore also having a border on the .is-selected version, but this border should be separate from focus — my instinct is 1px black. Jay if you get an opportunity to look at this, I'd appreciate it — otherwise I'll take a look when I have a chance.

@jameskoster
Copy link
Contributor Author

I think I have it:

select

I'm also trying cursor: default on hover as I find it a little distracting that the cursor and the outline change.

@jasmussen jasmussen requested a review from mtias November 26, 2020 13:49
@jameskoster jameskoster marked this pull request as ready for review November 26, 2020 13:52
@mtias
Copy link
Member

mtias commented Nov 26, 2020

Thanks for exploring! We might want to leave the outline visible on select as well until is-typing kicks in.

@karmatosed
Copy link
Member

karmatosed commented Nov 26, 2020

In exploring this it feels good as I interact. The hitches come as mentioned where certain behaviours outside the scope of this clash against the hover. This seems outside the scope here, but I mention as it could do with easing to ensure the experience flows as it can distract. For example:

image

Similarly here the issue with mis-aligned borders and numerous states is a little more obvious:

2020-11-26 15 44 45

@jasmussen
Copy link
Contributor

Seems related: #27075

@shaunandrews
Copy link
Contributor

This is a solid first step.

I think the subtle differences between 1px, 1.5px, 3px, blue, and black borders is going to be misunderstood by most people.

I find the current 1px, solid, blue line distracting but am willing to live with it as we improve as it does solve a number of real problems. I'd suggest moving away from box-shadow hacks and rounded corners, and sticking with the simple CSS outline for all block borders. Then, I can see using a dotted outlined to represent the hover state in a more subtle way.

Copy link
Member

@noahtallen noahtallen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it!

For some reason, the template part border is the same color for me. If that's intentional, then I'm curious why we need extra classes to handle borders for the template part block specifically? It seems like it could just be generic. I also agree with shaun re: not using box-shadows, but I'm ok moving forward with this to see how folks like it!

@@ -208,7 +208,6 @@

// Active entity spotlight.
&.has-active-entity:not(.is-focus-mode) {
opacity: 0.5;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are removing the template part spotlight mode thingy, we can probably remove this class entry altogether

@jameskoster
Copy link
Contributor Author

the template part border is the same color for me. If that's intentional, then I'm curious why we need extra classes to handle borders for the template part block specifically?

It's intentional for now, we might re-explore later. The only template part specific class I can see is to handle the offset on the :after, but it doesn't look like we really need that, so I'll remove for now.

@jameskoster
Copy link
Contributor Author

Updated based on your review @noahtallen, thanks for that. I also moved the outline styles to what I believe is a more appropriate file.

@Addison-Stavlo
Copy link
Contributor

Im curious why we are limiting this change to the site editor? Is the idea to try it out here and potentially move these over to the post editors if it proves out well?

It seems like a lot of the interactions this aims to improve are not site editor specific. While it is noted above that block nesting is featured more prominently in this context by default, it can still be just as (or more) prominent in the post editors based on the users needs and designs that are being implemented there.

@noahtallen
Copy link
Member

It'd be cool to have it as a setting. Maybe it would be off by default there, and on by default in the site editor. (maybe that could be a followup?)

@mtias
Copy link
Member

mtias commented Dec 1, 2020

By default, this effect could be jarring on the post editor alone (we tried it a while ago) overemphasizing blocks too much. I think eventually, if this iteration works out for the better, it should be part of the "select" tool. The site editor could make the select tool the default, then.

@jeyip
Copy link
Contributor

jeyip commented Dec 1, 2020

Testing

Requirements

I tested template parts and a variety of different blocks (image, paragraph, etc.)

  • Un-selected, hovered blocks display a 1px blue outline
  • Selected + focused blocks display a 1.5px blue outline (I clicked on a block directly)
  • Selected un-focussed blocks display a 1px black outline (I clicked a block and then clicked on block settings)

Browsers

  • Firefox
  • Chrome
  • IE11 (Side note the block settings button is not clickable in IE11)
  • Edge
  • Safari

I did notice a bit of wonky behavior with placeholders. @jasmussen pointed out issues with dead zones earlier, and I'm seeing them as well.
2020-12-01 01 06 10

Overall, I think the code changes look reasonable, and the editing experience, to me, definitely feels more intuitive than before. Thanks for working on this @jameskoster 🙌

@jameskoster
Copy link
Contributor Author

Thanks for testing @jeyip :) The 'dead zone' issue is reported here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Type] Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Full Site Editing: Block selection
8 participants