Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

Added option to use tel type for time fields #72

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

smoyte
Copy link

@smoyte smoyte commented Oct 6, 2016

Unless there is some reason not to? Thanks for considering!

@katemihalikova
Copy link
Owner

Hi Tom,
thanks for your contribution!

There was some discussion in #3, #42 and #43. It is not possible to use type="number" because the field features prefix zero which is not supported by that type. I've decided that we can use type="tel" as an optional feature (use some attr to switch type="text" to type="tel"). If you are interested, feel free to implement it :)

Thanks again for your work!

@smoyte smoyte changed the title Changed time popup fields to type=number Added option to use tel type for time fields Oct 6, 2016
@smoyte
Copy link
Author

smoyte commented Oct 6, 2016

How about now? I haven't tested this but it seems straightforward enough...

@katemihalikova
Copy link
Owner

This won't work as expected, because $scope.timeFieldType is a function, but it is used as a string value in the template. But we can simplify this more, see review notes.

@@ -13,7 +13,8 @@ angular.module("ion-datetime-picker", ["ionic"])
hourStep: "=?",
minuteStep: "=?",
secondStep: "=?",
onlyValid: "=?"
onlyValid: "=?",
telTypeForTimeFields: "=?"
Copy link
Owner

@katemihalikova katemihalikova Oct 7, 2016

Choose a reason for hiding this comment

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

Remove this line (because all boolean attrs are handled differently, see below).

@@ -281,6 +282,10 @@ angular.module("ion-datetime-picker", ["ionic"])
changeViewData();
};

$scope.timeFieldType = function() {
Copy link
Owner

@katemihalikova katemihalikova Oct 7, 2016

Choose a reason for hiding this comment

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

Remove this and instead use the same approach as for example mondayFirst attr on line 311, in the link function.

$scope.timeFieldType = "useTelType" in $attrs && $attrs.useTelType !== "false" ? "tel" : "text";

use-tel-type is shorter than tel-type-for-time-fields and IMHO still expresses its purpose.

@@ -51,23 +51,23 @@
<label class="col col-20">
<div class="item item-input">
<div>
<input type="text" ng-model="bind.hour" pattern="0?([01]?[0-9]|2[0-3])" ng-change="change('hour')" ng-blur="changed()" required>
<input type="{{timeFieldType}}" ng-model="bind.hour" pattern="0?([01]?[0-9]|2[0-3])" ng-change="change('hour')" ng-blur="changed()" required>
Copy link
Owner

Choose a reason for hiding this comment

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

Please use ng-attr-type= instead of type=, there were some problems in older browsers with it.

@@ -281,6 +282,10 @@ angular.module("ion-datetime-picker", ["ionic"])
changeViewData();
};

$scope.timeFieldType = function() {
return $scope.telTypeForTimeFields ? "tel" : "text";
Copy link
Owner

Choose a reason for hiding this comment

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

In the future, please try to maintain the existing code style, including indentation style, even if you think that it's weird and unusual :)

@smoyte
Copy link
Author

smoyte commented Nov 15, 2016

Haven't forgotten about this one! Just short on time ATM. Hope to finish this up within the year :)

@katemihalikova
Copy link
Owner

No problem, take your time 👍

@bettysteger
Copy link

Maybe you could also add

autocomplete="off" autocorrect="off" spellcheck="false"

to the input field, because I got an "ugly" autocorrect bar above the keyboard (KeyboardAccessoryBar) and autocomplete/autocorrect suggestions are not necessary when changing the time ⌛️ 😅

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

Successfully merging this pull request may close these issues.

3 participants