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

fix(toolbar): prevent content overflow and line-wrapping #2454

Merged
merged 3 commits into from
Feb 10, 2017

Conversation

devversion
Copy link
Member

  • No longer wraps lines inside of a md-toolbar-row
  • Overflow for X-axis and Y-Axis are now hidden. Developers should use multiple md-toolbar-row elements or can always overwrite the styles.

Note: An ellipsis for flex elements is definitely possible, but it just bloats the toolbar and needs some refactoring. We should revisit this with #1718

Fixes #2451

* No longer wraps lines inside of a `md-toolbar-row`
* Overflow for X-axis and Y-Axis are now hidden. Developers should use multiple `md-toolbar-row` elements or can always overwrite the styles.

Fixes angular#2451
@devversion devversion requested a review from jelbourn December 29, 2016 03:08
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 29, 2016

// Ensure toolbar content doesn't wrap and exceed the md-toolbar-row width. Ellipsis doesn't work on flex elements.
white-space: nowrap;
overflow: hidden;
Copy link
Member

Choose a reason for hiding this comment

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

There's a md-truncate-line mixin for these cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, but this also tries to show an ellipsis, which doesn't work and is then unnecessary.

@jelbourn
Copy link
Member

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Jan 13, 2017
@mmalerba
Copy link
Contributor

I'm seeing some test failures when I try to merge this into Google's codebase:

  1. The white-space: nowrap seems to affect the children of md-toolbar-row as well. I think flex-wrap: nowrap might work better in this case.

  2. One team has an absolutely positioned popup when you hover over a button in the md-toolbar-row, the overflow: hidden here doesn't play nice with that. Is there some way we can avoid it?

@mmalerba mmalerba removed the action: merge The PR is ready for merge by the caretaker label Jan 20, 2017
@devversion
Copy link
Member Author

As discussed with @mmalerba. We are just doing white-space: none for now because having overflow: hidden might reduce flexibility for toolbar elements.

@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Jan 20, 2017
@tinayuangao tinayuangao merged commit e728771 into angular:master Feb 10, 2017
@devversion devversion deleted the fix/toolbar-text-wrap branch February 10, 2017 05:51
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toolbar content overflow problem.
6 participants