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

Make the inserter full height #20526

Merged
merged 3 commits into from Mar 5, 2020
Merged

Make the inserter full height #20526

merged 3 commits into from Mar 5, 2020

Conversation

@youknowriad
Copy link
Contributor

youknowriad commented Feb 28, 2020

Alternative to #19836

Tries to accomplish the same result with smaller changes that work across inserters.

@github-actions

This comment has been minimized.

Copy link

github-actions bot commented Feb 28, 2020

Size Change: -346 B (0%)

Total Size: 865 kB

Filename Size Change
build/block-editor/index.js 104 kB -225 B (0%)
build/block-editor/style-rtl.css 10.5 kB -3 B (0%)
build/block-editor/style.css 10.5 kB -3 B (0%)
build/edit-post/index.js 90.8 kB -99 B (0%)
build/editor/index.js 44.6 kB -16 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.01 kB 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-library/editor-rtl.css 7.36 kB 0 B
build/block-library/editor.css 7.36 kB 0 B
build/block-library/index.js 116 kB 0 B
build/block-library/style-rtl.css 7.5 kB 0 B
build/block-library/style.css 7.51 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-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.6 kB 0 B
build/components/index.js 191 kB 0 B
build/components/style-rtl.css 15.6 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 5.76 kB 0 B
build/core-data/index.js 10.5 kB 0 B
build/data-controls/index.js 1.03 kB 0 B
build/data/index.js 8.22 kB 0 B
build/date/index.js 5.37 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.06 kB 0 B
build/edit-post/style-rtl.css 8.53 kB 0 B
build/edit-post/style.css 8.53 kB 0 B
build/edit-site/index.js 4.63 kB 0 B
build/edit-site/style-rtl.css 2.51 kB 0 B
build/edit-site/style.css 2.51 kB 0 B
build/edit-widgets/index.js 4.42 kB 0 B
build/edit-widgets/style-rtl.css 2.59 kB 0 B
build/edit-widgets/style.css 2.58 kB 0 B
build/editor/editor-styles-rtl.css 325 B 0 B
build/editor/editor-styles.css 327 B 0 B
build/editor/style-rtl.css 3.98 kB 0 B
build/editor/style.css 3.98 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.6 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 1.92 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.48 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.68 kB 0 B
build/list-reusable-blocks/index.js 2.99 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 4.85 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.02 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.49 kB 0 B
build/priority-queue/index.js 780 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.3 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@youknowriad youknowriad force-pushed the try/full-height-inserter branch from d51b208 to e940bf6 Feb 28, 2020
@youknowriad youknowriad marked this pull request as ready for review Feb 28, 2020
@youknowriad youknowriad requested a review from ellatrix as a code owner Feb 28, 2020
@youknowriad youknowriad self-assigned this Feb 28, 2020
@youknowriad youknowriad requested a review from jasmussen Feb 28, 2020
@youknowriad youknowriad force-pushed the try/full-height-inserter branch from e940bf6 to 3e8d524 Feb 28, 2020
@jasmussen

This comment has been minimized.

Copy link
Contributor

jasmussen commented Feb 28, 2020

Thanks for working on this. I imagine we'll want to use this as the basis going forward. Since working on the last one, I've been thinking that perhaps the treatment of blocks themselves might need a little more work, so I think it would be good to freshen up the mockup and then I can help bring this branch home.

@jasmussen jasmussen requested a review from mtias Feb 28, 2020
@youknowriad

This comment has been minimized.

Copy link
Contributor Author

youknowriad commented Feb 28, 2020

Noting that this PR is also a requirement before starting to explore showing the block patterns in the inserter.

@youknowriad

This comment has been minimized.

Copy link
Contributor Author

youknowriad commented Mar 3, 2020

Can we merge this and continue iteratively or what needs to be done here?

@jasmussen

This comment has been minimized.

Copy link
Contributor

jasmussen commented Mar 3, 2020

Took this branch for a spin, here's a GIF:

gif

I do think it's a superior experience, but I'd love @mtias thoughts on it before we merge. I believe he's been thinking about the inserters a lot lately, and might have thoughts.

If we do go forward with this branch (I'm a fan), then two things:

  1. There are a bunch of vertical scrollbars on the preview, not sure why exactly, probably something I can help fix if need be.
  2. Let's bid farewell to the initial help text. It's a ton of verbiage and once you've read it a single time, you need not read it again.
@enriquesanchez

This comment has been minimized.

Copy link
Contributor

enriquesanchez commented Mar 3, 2020

This feels vastly better than what we have. On an "averagely sized" window, it looks great:

Screen Shot 2020-03-03 at 17 21 37

I'm however not a fan of all the empty space when the window is taller than average:

Screen Shot 2020-03-03 at 17 22 11

@jasmussen not sure if you've put any thought on this. I myself I'm undecided on how to go about this and if it's a big issue at all.

@youknowriad I think I found a minor bug: when increasing my browser's window height, the inserter does not accurately grow in proportion to the window:

Screen Shot 2020-03-03 at 17 21 57

@jasmussen

This comment has been minimized.

Copy link
Contributor

jasmussen commented Mar 4, 2020

not sure if you've put any thought on this. I myself I'm undecided on how to go about this and if it's a big issue at all.

I think it's not a big issue, but I definitely hear you, it's something to consider.

The reason for the very tall inserter, outside of just leveraging the space, is because people don't scroll and discover more blocks. I would suggest that's an argument to explore collapsing only some of the block sections, like Embeds perhaps, and then defaulting more categories to open.

@youknowriad

This comment has been minimized.

Copy link
Contributor Author

youknowriad commented Mar 5, 2020

when increasing my browser's window height, the inserter does not accurately grow in proportion to the window:
@enriquesanchez This doesn't happen to me unless the dev tools are open which make me think it's fine.

@youknowriad youknowriad force-pushed the try/full-height-inserter branch from e2ec7c1 to b820f5f Mar 5, 2020
@youknowriad

This comment has been minimized.

Copy link
Contributor Author

youknowriad commented Mar 5, 2020

did some cleanup here and rebased, it should be good.

@karmatosed karmatosed self-requested a review Mar 5, 2020
Copy link
Member

karmatosed left a comment

Design wise, this does what expect it to. I will note that as the breadcrumbs become more important, potentially not overlapping those could be an iteration. That said, let's get this in so removing the design feedback label for now.

@youknowriad

This comment has been minimized.

Copy link
Contributor Author

youknowriad commented Mar 5, 2020

Let's try that.

@youknowriad youknowriad merged commit 34ed2a6 into master Mar 5, 2020
3 checks passed
3 checks passed
build
Details
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@youknowriad youknowriad deleted the try/full-height-inserter branch Mar 5, 2020
@github-actions github-actions bot added this to the Gutenberg 7.7 milestone Mar 5, 2020
@karmatosed karmatosed restored the try/full-height-inserter branch Mar 6, 2020
@mtias mtias deleted the try/full-height-inserter branch Mar 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.