Skip to content
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

refactor(android): improve parity for keyboardType #11134

Merged
merged 5 commits into from Mar 25, 2020

Conversation

ypbnv
Copy link
Contributor

@ypbnv ypbnv commented Aug 12, 2019

JIRA: https://jira.appcelerator.org/browse/TIMOB-17203

Description:
Refactor kyeboardType handling in order to get rid off redundant code and improve parity. Assign the TYPE_CLASS_TEXT only for text type input types.
Note: No unit tests since this is visual change.

@vijaysingh-axway Would you be able to check two things on the iOS related to the ticket - is the TYPE_NAMEPHONE_PAD constant used, because I see it is not taken into account in the Android code? Also is the expected behavior in iOS to be able to put multiple decimal points for TYPE_DECIMAL_PAD? That would not be a native android behavior. Setting the decimal tab flag only allows one decimal point.

Test case:
app.js

var win = Ti.UI.createWindow({layout:'vertical'}),
	textASCII = Ti.UI.createTextField({width: 300, keyboardType: Titanium.UI.KEYBOARD_TYPE_ASCII}),
	textDecimalPad = Ti.UI.createTextField({width: 300, keyboardType: Titanium.UI.KEYBOARD_TYPE_DECIMAL_PAD}),
	textDefault = Ti.UI.createTextField({width: 300, keyboardType: Titanium.UI.KEYBOARD_TYPE_DEFAULT}),
	textEmail = Ti.UI.createTextField({width: 300, keyboardType: Titanium.UI.KEYBOARD_TYPE_EMAIL}),
	textNamephonePad = Ti.UI.createTextField({width: 300, keyboardType: Titanium.UI.KEYBOARD_TYPE_NAMEPHONE_PAD}),
	textNumbersPunctuation = Ti.UI.createTextField({width: 300, keyboardType: Titanium.UI.KEYBOARD_TYPE_NUMBERS_PUNCTUATION}),
	textNumberPad = Ti.UI.createTextField({width: 300, keyboardType: Titanium.UI.KEYBOARD_TYPE_NUMBER_PAD}),
	textPhonePad = Ti.UI.createTextField({width: 300, keyboardType: Titanium.UI.KEYBOARD_TYPE_PHONE_PAD}),
	textURL = Ti.UI.createTextField({width: 300, keyboardType: Titanium.UI.KEYBOARD_TYPE_URL});
win.add([textASCII,
		textDecimalPad,
		textDefault,
		textEmail,
		textNamephonePad,
		textNumbersPunctuation,
		textNumberPad,
		textPhonePad,
		textURL]);
win.open();

TYPE_NUMBER_PAD and TYPE_DECIMAL_PAD should no more allow typing non-number characters, besides one decimal point for the latter.

Refactor kyeboardType handling in order to get rid off redundant code and improve parity
@build
Copy link
Contributor

build commented Aug 12, 2019

Messages
📖 👍 Hey!, You deleted more code than you added. That's awesome!
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖

✅ All tests are passing
Nice one! All 6634 tests are passing.
(There are 699 skipped tests not included in that total)

Generated by 🚫 dangerJS against 3032ede

@jquick-axway jquick-axway requested review from garymathews and removed request for jquick-axway August 15, 2019 20:59
private static final int KEYBOARD_NAMEPHONE_PAD = 6;
private static final int KEYBOARD_DEFAULT = 7;
private static final int KEYBOARD_DECIMAL_PAD = 8;

// UIModule also has these as values - there's a chance they won't stay in sync if somebody changes one without changing these
private static final int TEXT_AUTOCAPITALIZATION_NONE = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably remove these constants too, and use UIModule.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same for RETURNKEY_* constants

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed both blocks and pointed the constant to the UIModule namespace.

@sgtcoolguy
Copy link
Contributor

ping @ypbnv and @garymathews - have the changes satisfied the CR? Can we dismiss it and move to in qe testing?

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

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

CR: PASS

@ewanharris ewanharris modified the milestones: 9.0.0, 9.1.0 Feb 12, 2020
@vijaysingh-axway
Copy link
Contributor

Answer for iOS related query-

  1. KEYBOARD_TYPE_NAMEPHONE_PAD is used in iOS.
  2. KEYBOARD_TYPE_DECIMAL_PAD - It is possible to put multiple decimal point.

@ssekhri
Copy link

ssekhri commented Mar 25, 2020

FR Passed. The KEYBOARD_TYPE_NUMBER_PAD does not accept decimal point. The KEYBOARD_TYPE_DECIMAL_PAD is still different on iOS and Android as it accepts multiple points in iOS but just one in Android but these are by native behaviors.
Verified on:
Mac OS: 10.15.1
SDK: 9.1.0.v20200325104908
Appc CLI: 8.0.0
JDK: 11.0.4
Node: 10.17.0
Studio: 6.0.0.202003181504
Device: Nexus 4(v5.1)

@ssekhri ssekhri merged commit d72b004 into tidev:master Mar 25, 2020
sgtcoolguy pushed a commit that referenced this pull request Apr 9, 2020
* refactor(android): improve parity for keyboardType

Refactor kyeboardType handling in order to get rid off redundant code and improve parity

* refactor(android): point TiUIText to the proper constants namespace

Co-authored-by: Gary Mathews <contact@garymathews.com>
Co-authored-by: Samir Mohammed <ssjsamir@users.noreply.github.com>
Co-authored-by: Vijay Vikram Singh <vsingh@axway.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants