Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Fix(select): layout issue #7509

Closed

Conversation

david-meza
Copy link

  • Make selector less aggressive restricting it to the text span only
  • Add a max width, so the overflow rule will go in effect if we reach the width of the container
  • Allow span to shrink if width is greater than that of the input container

Closes #6200 and #6312

@david-meza
Copy link
Author

Can you also check I did the git procedure correctly? I don't think you want the merge with master message, but not entirely sure how to remove it. Do I just do git rebase <my-branch-name>?

@ThomasBurleson
Copy link
Contributor

@topherfangio - can you review plz

@topherfangio topherfangio self-assigned this Mar 10, 2016
@topherfangio topherfangio added the needs: review This PR is waiting on review from the team label Mar 10, 2016
@topherfangio
Copy link
Contributor

@david-meza Thanks for this PR! The code looks good to me, but can you squash your commits as outlined here: http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html

Make sure to pick the first commit and squash the "Merged ..." commit.

The commands you run should look something like:

git checkout master
git pull upstream master --rebase
git checkout fix/select-layout-issue
git rebase -i master
git push --force

Note that the force push will only update the branch you are on, so make sure you're still on the fix/select-layout-issue branch when you run it. That should update this PR with a single commit.

Let me know if you have questions!

@topherfangio topherfangio added needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved and removed needs: review This PR is waiting on review from the team labels Mar 10, 2016
@topherfangio topherfangio added this to the 1.1.1 milestone Mar 10, 2016
@david-meza
Copy link
Author

Thanks! That was helpful. Only got a bit confused because there's no --rebase option for pulling but eventually got it working.

transform: translate3d(0, 2px, 0);

._md-text {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this nested under & > span:not(._md-select-icon) ?
What is the use case ?

@ThomasBurleson ThomasBurleson added in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs and removed needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved labels Mar 11, 2016
@david-meza
Copy link
Author

That is a great question @ThomasBurleson . Sorry your comment got deleted in the rebase. Basically the reason why it was nested is because you should apply those rules to the element that actually contains the text, otherwise the parent would hide the overflow, but not apply the text-overflow rule. However, I later noticed that ._md-text had an inline display, so this rule should make the span have the behavior you want (because the div loses it's default block display, thus making the text being part of the span)

- Make selector less agressive restricting it to the text span only
- Add a max width, so the overflow rule will go in effect if we reach the width of the container
- Allow span to shrink if width is greather than that of the input container

Closes angular#6200 and angular#6312
@ThomasBurleson
Copy link
Contributor

@topherfangio - please review

@ThomasBurleson ThomasBurleson added needs: review This PR is waiting on review from the team and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs labels Mar 14, 2016
@ThomasBurleson ThomasBurleson modified the milestones: 1.1.0, 1.1.1 Mar 14, 2016
ThomasBurleson pushed a commit that referenced this pull request Mar 16, 2016
- Make selector less agressive restricting it to the text span only
- Add a max width, so the overflow rule will go in effect if we reach the width of the container
- Allow span to shrink if width is greather than that of the input container

Closes #6200 and #6312

Closes #7509
@angular angular deleted a comment from yanndupuy Nov 22, 2017
@angular angular deleted a comment from yanndupuy Nov 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: review This PR is waiting on review from the team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants