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 youknowriad commented Feb 28, 2020

Alternative to #19836

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

@github-actions
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 marked this pull request as ready for review February 28, 2020 09:24
@youknowriad youknowriad self-assigned this Feb 28, 2020
@youknowriad youknowriad added [Feature] Inserter The main way to insert blocks using the + button in the editing interface [Package] Block library /packages/block-library labels Feb 28, 2020
@jasmussen
Copy link
Contributor

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 February 28, 2020 09:51
@youknowriad
Copy link
Contributor Author

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

@youknowriad youknowriad added the Needs Design Feedback Needs general design feedback. label Feb 28, 2020
@youknowriad
Copy link
Contributor Author

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

@jasmussen
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@karmatosed karmatosed self-requested a review March 5, 2020 13:25
Copy link
Member

@karmatosed karmatosed left a comment

Choose a reason for hiding this comment

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

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.

@karmatosed karmatosed removed the Needs Design Feedback Needs general design feedback. label Mar 5, 2020
@youknowriad
Copy link
Contributor Author

Let's try that.

@youknowriad youknowriad merged commit 34ed2a6 into master Mar 5, 2020
@youknowriad youknowriad deleted the try/full-height-inserter branch March 5, 2020 15:18
@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 March 6, 2020 16:54
@mtias mtias deleted the try/full-height-inserter branch March 7, 2020 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Package] Block library /packages/block-library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants