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

Try: Nicer block footprint for social links. #20978

Merged
merged 2 commits into from
Mar 18, 2020

Conversation

jasmussen
Copy link
Contributor

Let the images speak. Before:

Screenshot 2020-03-18 at 10 04 29

After:

Screenshot 2020-03-18 at 10 22 38

@jasmussen jasmussen added the [Type] Enhancement A suggestion for improvement. label Mar 18, 2020
@jasmussen jasmussen requested a review from mkaz as a code owner March 18, 2020 09:23
@jasmussen jasmussen self-assigned this Mar 18, 2020
@@ -15,41 +15,10 @@
// @todo: eventually we may add a feature that lets a parent container absorb the block UI of a child block.
// When that happens, leverage that instead of the following overrides.
[data-type="core/social-links"] {
// 1. Reset margins on immediate innerblocks container.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All this red is dead CSS per the G2 effort.

@@ -114,3 +83,9 @@
// Windows High Contrast mode will show this outline, but not the box-shadow.
outline: 2px solid transparent;
}

// To ensure a better selection footprint when editing, attach the margin to the block container.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This CSS should become dead eventually, but in the mean time it improves the experience.

@github-actions
Copy link

github-actions bot commented Mar 18, 2020

Size Change: +268 B (0%)

Total Size: 857 kB

Filename Size Change
build/block-editor/index.js 100 kB +70 B (0%)
build/block-library/editor-rtl.css 7.21 kB -26 B (0%)
build/block-library/editor.css 7.21 kB -26 B (0%)
build/block-library/index.js 111 kB +20 B (0%)
build/block-serialization-default-parser/index.js 1.65 kB +1 B
build/blocks/index.js 57.5 kB -1 B
build/components/index.js 191 kB +5 B (0%)
build/compose/index.js 6.21 kB +2 B (0%)
build/core-data/index.js 10.6 kB -2 B (0%)
build/data/index.js 8.2 kB +1 B
build/deprecated/index.js 771 B -1 B
build/edit-post/index.js 91.2 kB -52 B (0%)
build/edit-post/style-rtl.css 8.47 kB -55 B (0%)
build/edit-post/style.css 8.46 kB -54 B (0%)
build/edit-site/index.js 5.56 kB +492 B (8%) 🔍
build/edit-site/style-rtl.css 2.62 kB +95 B (3%)
build/edit-site/style.css 2.62 kB +95 B (3%)
build/edit-widgets/index.js 4.43 kB +4 B (0%)
build/editor/index.js 43.8 kB -150 B (0%)
build/editor/style-rtl.css 3.97 kB -1 B
build/editor/style.css 3.96 kB -2 B (0%)
build/element/index.js 4.45 kB -3 B (0%)
build/format-library/index.js 6.95 kB -137 B (1%)
build/i18n/index.js 3.49 kB +1 B
build/is-shallow-equal/index.js 711 B +1 B
build/keyboard-shortcuts/index.js 2.3 kB -1 B
build/keycodes/index.js 1.69 kB +1 B
build/list-reusable-blocks/index.js 2.99 kB -1 B
build/media-utils/index.js 4.83 kB -3 B (0%)
build/redux-routine/index.js 2.83 kB -3 B (0%)
build/rich-text/index.js 14.3 kB -2 B (0%)
build/url/index.js 4.01 kB -1 B
build/viewport/index.js 1.61 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 998 B 0 B
build/annotations/index.js 3.43 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.02 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/style-rtl.css 10.9 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/style-rtl.css 7.42 kB 0 B
build/block-library/style.css 7.43 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.7 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/date/index.js 5.37 kB 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-widgets/style-rtl.css 2.58 kB 0 B
build/edit-widgets/style.css 2.58 kB 0 B
build/editor/editor-styles-rtl.css 381 B 0 B
build/editor/editor-styles.css 382 B 0 B
build/escape-html/index.js 733 B 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 1.93 kB 0 B
build/html-entities/index.js 621 B 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/notices/index.js 1.58 kB 0 B
build/nux/index.js 3.01 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.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 780 B 0 B
build/server-side-render/index.js 2.55 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@earnjam
Copy link
Contributor

earnjam commented Mar 18, 2020

Still seeing that wide rectangle on navigation mode. Here is a gif of me jumping around by tabbing.

Kapture 2020-03-18 at 6 16 26

Kinda curious why it switches between a square and then a circle on the highlight. Seems like the circle you can actually change the link by hitting enter, but both seem to have the same block toolbar and sidebar.

@jasmussen
Copy link
Contributor Author

Great catch on selection mode. I'll see if I can augment the hack a bit to address that.

Kinda curious why it switches between a square and then a circle on the highlight. Seems like the circle you can actually change the link by hitting enter, but both seem to have the same block toolbar and sidebar.

This is something we should absolutely improve. It's the difference between the block having focus, and the button inside the block having focus.

I suspect that we might be able to unify those two, so that focusing the button actually focuses the block itself, because the distinction (given there's only one button per block) does not seem valuable. The tricky part is that you still have to be able to click the button to open the URL dialog, which kind of suggests it has to be focusable as well. So ensuring a unification of these is both mouse and keyboardable is a bit tricky, but something to definitely improve.

@jasmussen
Copy link
Contributor Author

Pushed fix for selection mode.

focus

As is commented in the code, once the DOM is lighter for this block, which is one PR per block, that CSS can be removed. But it does improve the experience in the meantime.

@earnjam
Copy link
Contributor

earnjam commented Mar 18, 2020

@jasmussen did you push? Not seeing any new commits.

@jasmussen
Copy link
Contributor Author

I did push, but I forgot to commit 🤦‍♂

Thanks!

Copy link
Contributor

@earnjam earnjam 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 now!

Failed test seems unrelated, so I restarted that job.

@jasmussen
Copy link
Contributor Author

Thank you!

@jasmussen jasmussen merged commit 6f63dc3 into master Mar 18, 2020
@jasmussen jasmussen deleted the try/polish-social-links-footprint branch March 18, 2020 17:19
@github-actions github-actions bot added this to the Gutenberg 7.8 milestone Mar 18, 2020
@chrisvanpatten
Copy link
Contributor

Nice stuff!

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.

3 participants