-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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(text-field): Support for types- color, date, datetime-local, etc #2854
Conversation
Is this fine? Do I need fix something or make any changes? |
packages/mdc-textfield/foundation.js
Outdated
@@ -49,9 +48,15 @@ class MDCTextFieldFoundation extends MDCFoundation { | |||
return !this.isValid() && !this.isFocused_; | |||
} | |||
|
|||
/** @return {boolean} */ |
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.
Change to @private
and update the name to shouldAlwaysFloat_
.
/**
* @return {boolean}
* @private
*/
get shouldAlwaysFloat_() {
const type = this.getNativeInput_().type;
return ALWAYS_FLOAT_TYPES.indexOf(type) >= 0;
}
/** @return {boolean} */
get shouldFloat() {
return this.shouldAlwaysFloat_ || this.isFocused_ || !!this.getValue() || this.isBadInput_();
}
packages/mdc-textfield/foundation.js
Outdated
/** | ||
* @param {string} type The type to set on the input Element. | ||
*/ | ||
setType(type) { |
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.
Dynamically changing type isn't a common use case. This should be removed.
packages/mdc-textfield/foundation.js
Outdated
/** | ||
* @return {string} The type of the input Element. | ||
*/ | ||
getType() { |
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.
This can be removed.
@williamernest Sorry for the late response. I have made the changes you suggested. |
Codecov Report
@@ Coverage Diff @@
## master #2854 +/- ##
==========================================
+ Coverage 98.36% 98.36% +<.01%
==========================================
Files 120 120
Lines 5123 5125 +2
Branches 638 637 -1
==========================================
+ Hits 5039 5041 +2
Misses 84 84
Continue to review full report at Codecov.
|
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.
LGTM
@namannehra Sorry for the delay. Thanks for fixing this for us! |
Label will always float for the following types as they show some UI even if value is empty
Added
getType
andsetType
methods to foundation.setType
must be used to update type after initialisation.