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

fix(input): apply directives to only suitable inputs #6243

Closed
wants to merge 1 commit into from
Closed

fix(input): apply directives to only suitable inputs #6243

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions src/ng/directive/input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ var inputType = {
</doc:example>
*/
'text': textInputType,
'textarea': textInputType,
Copy link
Contributor

Choose a reason for hiding this comment

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

What? Afaik there is no input[type=textarea], only the textarea element.

We want to use textInputType for textarea elements, so...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, but at least in chrome (the only browser I tested with), the "type" property of a <textarea> is "textarea". This directive is applied to <textarea> elements as well as <input> elements, so this was added to catch those.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah I see. I'm not totally sure if that will work for IE, but I guess we'll see. (In review I usually scan the diff from top to bottom, hadn't noticed you were using the property rather than attribute yet ;)



/**
Expand Down Expand Up @@ -419,12 +420,7 @@ var inputType = {
</doc:protractor>
</doc:example>
*/
'checkbox': checkboxInputType,

'hidden': noop,
'button': noop,
'submit': noop,
'reset': noop
'checkbox': checkboxInputType
};

// A helper function to call $setValidity and return the value / undefined,
Expand Down Expand Up @@ -830,8 +826,11 @@ var inputDirective = ['$browser', '$sniffer', function($browser, $sniffer) {
require: '?ngModel',
link: function(scope, element, attr, ctrl) {
if (ctrl) {
(inputType[lowercase(attr.type)] || inputType.text)(scope, element, attr, ctrl, $sniffer,
$browser);
var type = element.prop('type');

if (inputType[type]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove the lowercase call? input types are case insensitive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should not have, my mistake. Chrome at least returns a lowercase string for the type property. Notice this now uses the type property rather than the type attribute. But for cross-browser compatibility, it might be a good idea to lowercase the type string.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually it "sort of" works for this case because you're using the property rather than the attribute, but my comment above explains why this can be a problem (for legacy browsers). @Blesh's date stuff is probably also going to be a problem if this change happens, due to modern desktop FF (among others) not supporting html5 date input types.

inputType[type](scope, element, attr, ctrl, $sniffer, $browser);
}
}
}
};
Expand Down