Try: Remove menu item min-width. #25218

jasmussen · 2020-09-10T11:05:03Z

Popovers have a min-width. Which means you get very wide popovers in some cases:

This PR tries removing that, so they look just right (sized according to the content inside):

So why was it added in the first place? Well, it was added back in #9086 because an auto-expanding URL field didn't resize it. With this PR I'm revisiting that, though, because: should a textfield ever expand a popover? Isn't it best those have fixed sizes? And currently, they do:

So, this PR would be a nice little win for toolbar dropdowns, and even a bit of cleanup. But it'd be good to test existing popovers to make sure they work as intended.

github-actions · 2020-09-10T11:14:51Z

Size Change: +63 B (0%)

Total Size: 1.21 MB

Filename	Size	Change
`build/block-editor/style-rtl.css`	11.1 kB	+17 B (0%)
`build/block-editor/style.css`	11.1 kB	+18 B (0%)
`build/components/index.js`	172 kB	+6 B (0%)
`build/components/style-rtl.css`	15.3 kB	+11 B (0%)
`build/components/style.css`	15.2 kB	+11 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.14 kB	0 B
`build/annotations/index.js`	3.78 kB	0 B
`build/api-fetch/index.js`	3.45 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-editor/index.js`	131 kB	0 B
`build/block-library/editor-rtl.css`	9.06 kB	0 B
`build/block-library/editor.css`	9.06 kB	0 B
`build/block-library/index.js`	146 kB	0 B
`build/block-library/style-rtl.css`	7.9 kB	0 B
`build/block-library/style.css`	7.9 kB	0 B
`build/block-library/theme-rtl.css`	837 B	0 B
`build/block-library/theme.css`	838 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/compose/index.js`	9.81 kB	0 B
`build/core-data/index.js`	12.5 kB	0 B
`build/data-controls/index.js`	772 B	0 B
`build/data/index.js`	8.77 kB	0 B
`build/date/index.js`	31.8 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.46 kB	0 B
`build/edit-navigation/index.js`	11.2 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.41 kB	0 B
`build/edit-post/style.css`	6.39 kB	0 B
`build/edit-site/index.js`	22.4 kB	0 B
`build/edit-site/style-rtl.css`	3.88 kB	0 B
`build/edit-site/style.css`	3.88 kB	0 B
`build/edit-widgets/index.js`	26.4 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`	480 B	0 B
`build/editor/editor-styles.css`	482 B	0 B
`build/editor/index.js`	43.1 kB	0 B
`build/editor/style-rtl.css`	3.85 kB	0 B
`build/editor/style.css`	3.85 kB	0 B
`build/element/index.js`	4.65 kB	0 B
`build/escape-html/index.js`	735 B	0 B
`build/format-library/index.js`	7.7 kB	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.16 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`	712 B	0 B
`build/keyboard-shortcuts/index.js`	2.52 kB	0 B
`build/keycodes/index.js`	1.94 kB	0 B
`build/list-reusable-blocks/index.js`	3.11 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.34 kB	0 B
`build/notices/index.js`	1.79 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.85 kB	0 B
`build/reusable-blocks/index.js`	3.06 kB	0 B
`build/rich-text/index.js`	13.2 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`	4.06 kB	0 B
`build/viewport/index.js`	1.84 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.22 kB	0 B

_{compressed-size-action}

ItsJonQ · 2020-09-16T13:08:27Z

@jasmussen Woo! Thanks for this one ❤️

I just tested it locally. I've noticed some issues with some of the Toolbar dropdowns (for the Cover block):

This could perhaps be resolved by adding white-space: nowrap on menu items/buttons (which they perhaps should have to begin with 🤔 )

This leads to another issue, at least if the dropdown happens to have overflow 🙈 :

I gave that a shot. It worked (at establishing a better min-width). The other issue is that the icon is really close to the text (almost touching). This echos an issue with the Button/MenuItem icon setup, rather than min-width.

With all of that being said!! (Phew).
I don't think removing min-width is a causing these issues. If anything, I think it reveals some underlying UI issues.

(Not really sure how deep the rabbit hole goes on this one, but maybe worth taking a look?).

Ultimately though, I agree with your original mission in that, I too believe Dropdown menu widths should be more adaptive/flexible.

jasmussen · 2020-09-17T09:45:49Z

Excellent sleuthing, that was really helpful! I found a hardcoded 200px somewhere, which I changed to a min-width. Now it works:

But now this one is a bit out there, so I'll look into that now :D

jasmussen · 2020-09-17T09:57:20Z

Got the ellipsis menu working, alas it required another wrapper around menu items, but I think that's okay.

jasmussen · 2020-09-17T10:02:04Z

Okay I think I have it.

ItsJonQ · 2020-09-17T14:23:56Z

@jasmussen 🎉 ! Looks like it's working :).
The test suites are a bit unhappy... most likely due to the new added selector.
Going to poke at those to see if I can make them happy again.

ItsJonQ · 2020-09-17T15:05:28Z

🙈
I'm struggling with running E2E tests locally. For some reason, this is happening:

console.error
    Failed to load resource: net::ERR_PROXY_CONNECTION_FAILED

      170 |
      171 | 		// eslint-disable-next-line no-console
    > 172 | 		console[ logFunction ]( text );
          | 		^
      173 | 	} );
      174 | }
      175 |

      at CustomConsole.<anonymous> (../../node_modules/jest-environment-node/node_modules/jest-mock/build/index.js:814:25)
      at Page.<anonymous> (config/setup-test-framework.js:172:3)
      at Page._onLogEntryAdded (../../node_modules/puppeteer/lib/Page.js:203:18)

When I try to run

npm run test-e2e:watch -- packages/e2e-tests/specs/editor/various/reusable-blocks.test.js

(I tried blowing away Docker's resources and starting from scratch. It didn't seem to help)

Don't let me be a blocker for this one! If anyone else can help with E2E tests, by all means ❤️

jasmussen · 2020-09-17T16:00:03Z

Thanks so much for looking and fixing! I'll have another stab, and otherwise we'll find a way.

jasmussen · 2020-09-18T07:31:12Z

I pushed a few tweaks and rebased/squashed. Let's see if it does anything.

jasmussen · 2020-10-14T13:30:09Z

I could use help with this one. @gziolo would you have any insights and/or time?

gziolo · 2020-10-14T13:54:51Z

It looks like some e2e tests that use XPath need to be updated to reflect changes in HTML syntax.

gziolo · 2020-10-21T17:29:30Z

package-lock.json

@@ -46068,7 +46068,7 @@
 			"dependencies": {
 				"clone-deep": {
 					"version": "0.2.4",
-					"resolved": "https://registry.npmjs.org/clone-deep/-/clone-deep-0.2.4.tgz",
+					"resolved": "http://registry.npmjs.org/clone-deep/-/clone-deep-0.2.4.tgz",


I guess it shouldn't be in this PR. We can clean it up later...

gziolo · 2020-10-22T08:17:37Z

I see similar e2e test failures in the master branch. It looks like it shouldn't block this PR.

jasmussen · 2020-10-22T08:23:19Z

Since this PR is not urgent, I'll let it sit until the failures in the master branch are taken care of, then I'll rebase and we can review again. Thanks so much for all the help @gziolo !

mkaz

Tests are now passing and works as described. 👍

I noticed the properly sized drop down after applying the patch and did not find any regressions.

jasmussen · 2020-11-05T08:08:45Z

Awesome, let's try this, and I'll follow up and re-apply min-widths in specific cases if they turn out to be necessary!

jasmussen added the [Type] Enhancement A suggestion for improvement. label Sep 10, 2020

jasmussen requested review from ajitbohra, chrisvanpatten and youknowriad as code owners September 10, 2020 11:05

jasmussen self-assigned this Sep 10, 2020

jasmussen requested a review from ItsJonQ September 16, 2020 06:37

jasmussen force-pushed the try/remove-menu-item-minwidth branch from e0a0e50 to 418e223 Compare September 17, 2020 09:06

jasmussen requested a review from ellatrix as a code owner September 17, 2020 10:01

jasmussen force-pushed the try/remove-menu-item-minwidth branch 2 times, most recently from 41b585d to 070374d Compare September 18, 2020 07:15

jasmussen force-pushed the try/remove-menu-item-minwidth branch from e59f14c to a0f3a3a Compare October 20, 2020 08:37

gziolo force-pushed the try/remove-menu-item-minwidth branch from e609bf5 to 8d7810a Compare October 21, 2020 17:22

jasmussen requested review from gziolo, nerrad, ntwb and talldan as code owners October 21, 2020 17:22

gziolo reviewed Oct 21, 2020

View reviewed changes

gziolo force-pushed the try/remove-menu-item-minwidth branch from 8d7810a to a636e57 Compare October 21, 2020 18:36

jasmussen force-pushed the try/remove-menu-item-minwidth branch 4 times, most recently from bc3bfac to 303f222 Compare October 30, 2020 08:14

jasmussen and others added 6 commits November 4, 2020 13:19

Try: Remove menu item min-width.

e36dbc5

Followup menu item polish.

3ef7b07

Update MenuItem snapshot

4b59cc1

Polish more menu in block and top toolbar.

0917369

Fix remnant.

3b7aa94

Tests: Fix failing e2e tests that use menu items

6c77439

jasmussen force-pushed the try/remove-menu-item-minwidth branch from 303f222 to 6c77439 Compare November 4, 2020 12:21

mkaz approved these changes Nov 4, 2020

View reviewed changes

jasmussen merged commit 74fd69f into master Nov 5, 2020

jasmussen deleted the try/remove-menu-item-minwidth branch November 5, 2020 08:08

github-actions bot added this to the Gutenberg 9.4 milestone Nov 5, 2020

This was referenced Nov 5, 2020

Tweak some menu items. #26720

Merged

Try: Don't wrap menu items. #26889

Merged

stokesman mentioned this pull request Nov 26, 2020

Fix the image block's zoom slider width #27285

Merged

6 tasks

andrewserong mentioned this pull request Aug 30, 2021

DateTime Component: Fix sizing of help info #34370

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try: Remove menu item min-width. #25218

Try: Remove menu item min-width. #25218

jasmussen commented Sep 10, 2020

github-actions bot commented Sep 10, 2020 •

edited

Loading

ItsJonQ commented Sep 16, 2020

jasmussen commented Sep 17, 2020 •

edited

Loading

jasmussen commented Sep 17, 2020

jasmussen commented Sep 17, 2020

ItsJonQ commented Sep 17, 2020

ItsJonQ commented Sep 17, 2020

jasmussen commented Sep 17, 2020

jasmussen commented Sep 18, 2020

jasmussen commented Oct 14, 2020

gziolo commented Oct 14, 2020

gziolo Oct 21, 2020

gziolo commented Oct 22, 2020

jasmussen commented Oct 22, 2020

mkaz left a comment

jasmussen commented Nov 5, 2020

Try: Remove menu item min-width. #25218

Try: Remove menu item min-width. #25218

Conversation

jasmussen commented Sep 10, 2020

github-actions bot commented Sep 10, 2020 • edited Loading

ItsJonQ commented Sep 16, 2020

jasmussen commented Sep 17, 2020 • edited Loading

jasmussen commented Sep 17, 2020

jasmussen commented Sep 17, 2020

ItsJonQ commented Sep 17, 2020

ItsJonQ commented Sep 17, 2020

jasmussen commented Sep 17, 2020

jasmussen commented Sep 18, 2020

jasmussen commented Oct 14, 2020

gziolo commented Oct 14, 2020

gziolo Oct 21, 2020

Choose a reason for hiding this comment

gziolo commented Oct 22, 2020

jasmussen commented Oct 22, 2020

mkaz left a comment

Choose a reason for hiding this comment

jasmussen commented Nov 5, 2020

github-actions bot commented Sep 10, 2020 •

edited

Loading

jasmussen commented Sep 17, 2020 •

edited

Loading