-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
feat(select): make select work inside form-field #6488
Merged
+1,038
−954
Merged
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
4656ab4
strip down select DOM and implement MdFormFieldControl
mmalerba 80b65d5
tweak select styles to fit form-field
mmalerba 8ff539f
make dropdown adjust to form field's font size (still 1px rounding erorr
mmalerba 9ef3503
fix 1px jitter and baseline alignment
mmalerba 21c6701
finish up MdFormFieldControl implementation
mmalerba 272a762
fix floating placehodler and color
mmalerba bb43366
fix overlay font size on safari
mmalerba 6abca5a
fix width on firefox
mmalerba fd27cd1
open select when form field is clicked
mmalerba face17e
separate the "floating" logic from the "empty" logic
mmalerba 327c04b
fix slight error in multi-select dropdown placement
mmalerba 9626eb1
exclude hint and error from focusing select on click
mmalerba d1132f2
add some fancy form-field features to select demo
mmalerba 2d4cb62
add a class to form-field depending on the control type for easy styling
mmalerba b660ddf
fix most tests, exceptions noted with TODOs
mmalerba e0d2c06
fix prerender and input test
mmalerba 07e15a2
make placeholder float when select is opened, and prevent it from ove…
mmalerba db019d2
fix remaining tests
mmalerba 9c9e932
fix lint
mmalerba 6ec117e
clean up measurement logic
mmalerba 9cc3837
fix rebase issues
mmalerba 9c3f905
fix remaining demos and examples
mmalerba c8fadfd
fix some firefox issues
mmalerba 6348841
ignore rounding error in IE
mmalerba 02cd9d3
update new select instance that was added
mmalerba 16b2172
update md-chip-list implementation of MdFormFieldControl
mmalerba 11e7549
fix issue
mmalerba File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,4 +7,8 @@ | |
margin: 24px; | ||
} | ||
|
||
.demo-drink-icon { | ||
vertical-align: bottom; | ||
padding-right: 0.25em; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API-wise, do we really want people to have to wrap every select in a form field? E.g. if you were using option groups, you'd have to write 4 levels of HTML. Aside from having to proxy a few
@Input
-s, what are the cons of having the form field be consumed bymd-select
internally?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the past we've preferred composition style APIs since they're more flexible, even if they are a bit more verbose. I think the extra typing is worth it, but it's a fair point. @jelbourn do you have an opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do like the consistency of having both
select
andinput
inside the sameform-field
; it makes it clear that the prefix, suffix, hint, and error belong to the form field. Very explicit, no magic. It's also more maintainable for us, since we don't have to forward along everyform-field
API through select over time.I don't feel incredibly strongly about it, though. @mmalerba up to you if you think it would be the same amount of work to do it the other way around; it would certainly make the migration easier