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

Select Tool: Show block borders on :hover while using the Select tool #22508

Merged
merged 5 commits into from
Jun 11, 2020

Conversation

shaunandrews
Copy link
Contributor

@shaunandrews shaunandrews commented May 20, 2020

This PR adds a border to blocks while hovering with the Select tool active:

@github-actions
Copy link

github-actions bot commented May 20, 2020

Size Change: +235 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-editor/index.js 107 kB +101 B (0%)
build/block-editor/style-rtl.css 12.1 kB +66 B (0%)
build/block-editor/style.css 12.1 kB +67 B (0%)
build/block-library/index.js 129 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.22 kB 0 B
build/block-directory/style-rtl.css 892 B 0 B
build/block-directory/style.css 892 B 0 B
build/block-library/editor-rtl.css 7.88 kB 0 B
build/block-library/editor.css 7.89 kB 0 B
build/block-library/style-rtl.css 7.96 kB 0 B
build/block-library/style.css 7.96 kB 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 195 kB 0 B
build/components/style-rtl.css 19.5 kB 0 B
build/components/style.css 19.5 kB 0 B
build/compose/index.js 9.31 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.17 kB 0 B
build/edit-navigation/index.js 8.26 kB 0 B
build/edit-navigation/style-rtl.css 975 B 0 B
build/edit-navigation/style.css 974 B 0 B
build/edit-post/index.js 303 kB 0 B
build/edit-post/style-rtl.css 5.6 kB 0 B
build/edit-post/style.css 5.6 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 2.96 kB 0 B
build/edit-site/style.css 2.96 kB 0 B
build/edit-widgets/index.js 9.34 kB 0 B
build/edit-widgets/style-rtl.css 2.4 kB 0 B
build/edit-widgets/style.css 2.4 kB 0 B
build/editor/editor-styles-rtl.css 423 B 0 B
build/editor/editor-styles.css 423 B 0 B
build/editor/index.js 44.8 kB 0 B
build/editor/style-rtl.css 4.26 kB 0 B
build/editor/style.css 4.27 kB 0 B
build/element/index.js 4.64 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@mapk
Copy link
Contributor

mapk commented May 20, 2020

Shaun! This is looking pretty good. I've got some feedback after testing this out.

1. Border jumping

There's a weird action while hovering where the border jumps.

jump

2. Border contrast

The borders should adapt to allow proper color contrast levels depending on the background.

borders

3. Nested blocks

Should the user see every block they hover over? Or just the immediate child block border? Right now, as in the above gif, the user only would see the immediate child block's border.

Related: #22383

@mapk mapk added the Needs Design Feedback Needs general design feedback. label May 20, 2020
@ZebulanStanphill ZebulanStanphill added the [Type] Enhancement A suggestion for improvement. label May 20, 2020
@mtias
Copy link
Member

mtias commented May 21, 2020

Let's try to aim for just one block showing borders at a time.

@ZebulanStanphill
Copy link
Member

Yeah, for simplicity, let's just show the hovered block's border. We can always change it later, and just getting this in will be a nice enhancement. I know there are some people who have complained that they can't see the borders of blocks, and this should at least help a little if not a lot.

I also think adding this hover interaction makes a lot of sense for a "select" mode. You're trying to select a block, so it makes sense that you would want to get an idea of which block you're about to select by seeing its borders. This will make Select mode a lot more useful for mouse users.

@shaunandrews
Copy link
Contributor Author

With some help from @ellatrix I managed to get this working so that only a single block border appears as you move your cursor around. Here's how its looking now:

select-hover-border

--

There's a weird action while hovering where the border jumps.

I've no clue what's causing this, but its less noticeable with the recent updates. I think its something to do with the shape of the block's element changing for some unknown reason.

The borders should adapt to allow proper color contrast levels depending on the background.

I'm not so sure. The current select border doesn't really adapt, and if you compare with tools like Figma or Sketch the border color is consistent regardless of colors on the canvas.

--

I do notice that with the Select tool, the borders don't display for blocks nested within a Group block. However, this seems to be related to the Select tool itself — if I enable the CSS styles for the Edit tool, I'm able to highlight nested blocks just fine.

@shaunandrews shaunandrews added the Needs Design Feedback Needs general design feedback. label Jun 3, 2020
@enriquesanchez
Copy link
Contributor

Just took this for a spin and it feels good. I particularly like how the hover border is very thin and neutral.

Is there any reason why this hover effect is only available on the Select mode? I feel this will be useful no matter what mode you're on.

@mtias
Copy link
Member

mtias commented Jun 4, 2020

This last version looks to be arriving at a good place.

@ZebulanStanphill
Copy link
Member

Is there any reason why this hover effect is only available on the Select mode? I feel this will be useful no matter what mode you're on.

We used to have block borders shown on hover all the time, but it was removed to make the editor look more like the front-end by default (only the selected block should have its appearance modified) and to reduce UI flashing as you move your cursor around. It was part of the whole G2 effort to lighten the UI.

I think it's a good idea to bring back hover borders in Select mode, though, since that mode is specifically designed for selecting blocks, and showing borders makes a lot of sense in that context.

@ZebulanStanphill
Copy link
Member

I just tried out this branch. It seems to be working pretty well. The only thing I'm not sure about is the color of the hover border. It seems maybe a bit too light?

@shaunandrews shaunandrews marked this pull request as ready for review June 5, 2020 16:22
@shaunandrews shaunandrews requested a review from youknowriad as a code owner June 5, 2020 16:22
@shaunandrews
Copy link
Contributor Author

shaunandrews commented Jun 8, 2020

The SCSS variable I used, $light-gray-ui, is listed as part of the recent G2 work in the _colors.scss file.

It felt good to me when using. The name of the variable seems to match to intention here, but I'd be happy to change the hover color to something else.

@karmatosed
Copy link
Member

For me the light-gray works as it is just enough to support the movement. I don't think it needs to be high contrast beyond that, but that could be explored as an iteration if proven to be a need. The color is less important in strength than the movement with an indicator.


.is-navigate-mode & .block-editor-block-list__block.is-hovered:not(.is-selected)::after {
Copy link
Member

Choose a reason for hiding this comment

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

Would it be ok to only apply the hovered class in navigation mode? This might make edit mode a bit more performant, if the mouse listeners are gone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, that'd probably work just fine I think.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I changed its so the class is only added in navigation mode. So this .is-navigate-mode can be removed here and above.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps the class name should be updated to reflect that it isn't applied every time the block is hovered, but only when it's hovered in navigation/select mode?

@shaunandrews shaunandrews removed the Needs Design Feedback Needs general design feedback. label Jun 8, 2020
Copy link
Member

@ellatrix ellatrix left a comment

Choose a reason for hiding this comment

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

Looks good to me, if the colour doesn't need updating.

@ZebulanStanphill
Copy link
Member

I'm not opposed to the current color, so feel free to merge if you want to!

@shaunandrews shaunandrews force-pushed the try/select-mode-block-hover-outlines branch from 56bd95a to 92caa88 Compare June 11, 2020 18:24
@shaunandrews shaunandrews merged commit 7075e3a into master Jun 11, 2020
@shaunandrews shaunandrews deleted the try/select-mode-block-hover-outlines branch June 11, 2020 21:51
@github-actions github-actions bot added this to the Gutenberg 8.4 milestone Jun 11, 2020
This was referenced Jun 24, 2020
@noahtallen
Copy link
Member

noahtallen commented Aug 6, 2020

Is this supposed to still be working? :p

I am not seeing it work for any blocks locally currently. Not sure if bug or if it was removed on purpose!

edit: this might also just be a bug of what I'm working on

@ZebulanStanphill
Copy link
Member

@noahtallen It's working for me on the latest master.

image

@noahtallen
Copy link
Member

Thanks! I think it is an issue with what I'm working on. The changes seemed unrelated which is why I started looking here :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants