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

Prevent menu content from wrapping by default. Fixes #56. #102

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JanMiksovsky
Copy link

By turning off wrapping, items don't reflow during menu animation.

Note that, while most people will probably welcome this change, there's some risk that people have created dropdown menus that rely on wrapping.

This PR currently doesn't include a unit test, since it's exclusively styling, but let me know if one should be added. Along those same lines, I'd like to provide an update to the original repro case provided so that people can verify the fix. I had difficulty creating a jsBin that imported paper-menu-button.html off this fork, but imported all other dependencies off of polygit. If there's an easy way to do that, please let me know and I'll post an updated repro.

@@ -93,6 +93,11 @@
@apply(--paper-menu-button-content);
}

.dropdown-content ::content > * {
/* Ensure items are full width so they don't wrap during animation. */
white-space: nowrap;
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps it makes aesthetic sense to also truncate with ellipsis?

Copy link
Author

Choose a reason for hiding this comment

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

I'd recommend against doing that.

The menu is only animated for a short time, not long enough to read the text of the long menu items. By the time someone can recognize that an ellipsis indicates that some of the menu item text is currently hidden, the complete menu item text will be shown. During that interval, showing ellipsis briefly would just constitute another source of flicker, when it's the intent of this PR to reduce flicker during the animation.

Moreover, ellipses are conventionally used at the ends of menu items with a specific semantic intent: to signal that a menu item will open a dialog. Applying ellipses automatically to arbitrary menu items could lead to confusion.

@cdata
Copy link
Contributor

cdata commented Sep 6, 2016

Polygit allows for some basic configuration (docs). You can include your fork using a configuration like this:

<base href="//polygit.org/paper-menu-button+janmiksovsky/components/">
<link rel="import" href="../paper-menu-button/paper-menu-button.html">

@JanMiksovsky
Copy link
Author

JanMiksovsky commented Sep 6, 2016

Thanks for the pointer to the polygit docs.

I've posted a fiddle that uses this PR. Contrast with the original fiddle. In the original, the last item wraps during the animation. With this PR, the last item no longer wraps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants