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

Make md-select work more like md-input-container/input #3307

Closed
wants to merge 12 commits into from

Conversation

MagicIndustries
Copy link
Contributor

Adds required md-input-container to md-select.
Replaces md-select-label with md-select-value
Adjusts scss to make label transition the same way as with md-input-container/md-input
Removed md-select blacklist from placeholder

@thvd
Copy link

thvd commented Jun 17, 2015

I think we get a much nicer diff when you revert the 4 space indent back to the 2 space indent.

@MagicIndustries
Copy link
Contributor Author

@thvd Just noticed that, I'm not sure why it's gone in that way, it's 2 spaces in my IDE. I just cloned the repo, added my changes and re-committed. Shouldn't be anything adding extra spaces - I'm on OSX.

I just checked, in my fork it's two spaces as well. Did I accidentally tick a box somewhere when making the pull request?

@MagicIndustries
Copy link
Contributor Author

Ah, I was checking select, it's the others that were messed up. I'll re-send the pull request.

@MagicIndustries
Copy link
Contributor Author

Never mind, just realised the commit was added to the request :)

@nantunes
Copy link
Contributor

You should also try to keep with the project's code style. The diff is polluted with whitespace surrounding the function declarations.

Meanwhile, a cleaner diff: https://github.com/angular/material/pull/3307/files?w=1

Looking forward to this enhancement. 👍

@MagicIndustries
Copy link
Contributor Author

Thanks for the feedback @nantunes, I've updated the whitespace to remove as much of that noise as possible.

@dmackerman
Copy link

I'm all about this. I've always been confused by the lack of <md-input-container> with selects.

@dholcombe
Copy link

Thanks for doing this. I just implemented something similar in my site (the CSS portions anyhow). This more complete solution will be helpful when it ships.

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

Successfully merging this pull request may close these issues.

5 participants