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

Add an outline mode and use it both Site Editor and Template mode #27499

Merged
merged 3 commits into from
Dec 4, 2020

Conversation

youknowriad
Copy link
Contributor

This PR moves the Adhoc outlines revealing implemented in edit-site into a dedicated mode of the block-editor package and uses that mode in both Site Editor and the template mode of the post editor.

@youknowriad youknowriad added [Feature] Full Site Editing [Type] Experimental Experimental feature or API. [Type] New API New API to be used by plugin developers or package users. labels Dec 4, 2020
@youknowriad youknowriad self-assigned this Dec 4, 2020
@youknowriad youknowriad requested a review from ellatrix as a code owner December 4, 2020 09:28
@github-actions
Copy link

github-actions bot commented Dec 4, 2020

Size Change: -298 B (0%)

Total Size: 1.19 MB

Filename Size Change
build/block-editor/index.js 128 kB -4 B (0%)
build/block-editor/style-rtl.css 11.2 kB +56 B (0%)
build/block-editor/style.css 11.2 kB +53 B (0%)
build/edit-site/index.js 24.1 kB +7 B (0%)
build/edit-site/style-rtl.css 3.91 kB -156 B (3%)
build/edit-site/style.css 3.91 kB -155 B (3%)
build/editor/index.js 43.6 kB +18 B (0%)
build/format-library/index.js 6.74 kB -117 B (1%)
ℹ️ 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-library/editor-rtl.css 9.07 kB 0 B
build/block-library/editor.css 9.07 kB 0 B
build/block-library/index.js 149 kB 0 B
build/block-library/style-rtl.css 8.34 kB 0 B
build/block-library/style.css 8.34 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 15.2 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.49 kB 0 B
build/edit-post/style.css 6.47 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/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/element/index.js 4.63 kB 0 B
build/escape-html/index.js 735 B 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.57 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

@jasmussen
Copy link
Contributor

There's a lot of CSS removed here, and when testing it I'm not seeing the exact same behavior of outlines in the site editor. The heuristic we decided to test for starters was:

  • Hover = 1px blue
  • Selected & focused = 1.5px blue
  • Selected but not focused = 1px black

I'd like Jay who worked on this to test it a bit more indepthly in case I'm missing something, but it feels different.

This is purely an under the hood change, correct? Not surfaced in any prefs menu right?

@youknowriad
Copy link
Contributor Author

Yes, under the hood and the removal of CSS is because for me it didn't feel like it deserved a dedicated style while we already had the style (the one we use for select mode)

@jameskoster
Copy link
Contributor

I believe the per-state visual treatments Joen mentioned are an a11y requirement, so probably best to retain them.

The only other thing I see missing is the cursor: default style on .is-hovered.

@jasmussen
Copy link
Contributor

It's more a matter of us intentionally wanting to try out that new style as discussed in #27271. While there's always room for improvement, it's barely been tested yet and I think it's way too early to decide it didn't work.

@youknowriad
Copy link
Contributor Author

I can restore the styles but to be honest, I didn't even notice the difference myself so I really doubt we need any particular treatment here

@jasmussen
Copy link
Contributor

I'm recording some demo work right now, and I've found a couple of things I want to tweak:

But despite that, it feels like a really good balance that I think can be polished to work very well. I think we should keep it and refine it, and revisit in a few releases. Thank you Riad.

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Code LGTM. I don't see any visual change from master and works beautifully. Designers are better judges from me on the styling though...

Thanks Riad 👍

@@ -122,6 +122,7 @@ function BlockListBlock( {
isDragging: isBlockBeingDragged( clientId ),
isHighlighted: isBlockHighlighted( clientId ),
isFocusMode: getSettings().focusMode,
isOutlineMode: getSettings().outlineMode,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick - call getSettings once and extract props?

@youknowriad youknowriad merged commit 9d87c64 into master Dec 4, 2020
@youknowriad youknowriad deleted the add/outline-mode branch December 4, 2020 14:37
@github-actions github-actions bot added this to the Gutenberg 9.6 milestone Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Experimental Experimental feature or API. [Type] New API New API to be used by plugin developers or package users.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants