Titanium JIRA Archive
Titanium SDK/CLI (TIMOB)

[TIMOB-24866] Android: Setting a TextField's "padding" resets its alignment to left/center

GitHub Issuen/a
TypeBug
PriorityMedium
StatusClosed
ResolutionFixed
Resolution Date2017-11-15T04:31:52.000+0000
Affected Version/sRelease 6.0.2
Fix Version/sRelease 7.0.0
ComponentsAndroid
Labelsandroid, textfield
ReporterJoshua Quick
AssigneeGary Mathews
Created2017-06-20T21:56:27.000+0000
Updated2017-11-15T06:46:51.000+0000

Description

*Summary:* Setting the padding on a TextField wrongly resets its horizontal "textAlign" and "verticalAlign" properties to left and center respectively. *Steps to reproduce:*

Build and run the below code on Android.

Tap the "Set Padding" button.

Notice that the text alignment of the 3 fields change to left/center.

var window = Ti.UI.createWindow({ layout: "vertical" });

var textField1 = Ti.UI.createTextField(
{
	value: "Top-Left Aligned Text",
	textAlign: Ti.UI.TEXT_ALIGNMENT_LEFT,
	verticalAlign: Ti.UI.TEXT_VERTICAL_ALIGNMENT_TOP,
	backgroundColor: "black",
	width: "90%",
	height: "20%",
	top: "5%",
});
window.add(textField1);

var textField2 = Ti.UI.createTextField(
{
	value: "Centered Text",
	textAlign: Ti.UI.TEXT_ALIGNMENT_CENTER,
	verticalAlign: Ti.UI.TEXT_VERTICAL_ALIGNMENT_CENTER,
	backgroundColor: "black",
	width: "90%",
	height: "20%",
	top: "5%",
});
window.add(textField2);

var textField3 = Ti.UI.createTextField(
{
	value: "Bottom-Right Aligned Text",
	textAlign: Ti.UI.TEXT_ALIGNMENT_RIGHT,
	verticalAlign: Ti.UI.TEXT_VERTICAL_ALIGNMENT_BOTTOM,
	backgroundColor: "black",
	width: "90%",
	height: "20%",
	top: "5%",
});
window.add(textField3);

var button = Ti.UI.createButton(
{
	title: "Set Padding",
	top: "5%",
});
button.addEventListener("click", function(e)
{
	var padding = { top: 5, left: 5, bottom: 5, right: 5 };
	textField1.padding = padding;
	textField2.padding = padding;
	textField3.padding = padding;
});
window.add(button);

window.open();
*Expected Result:* Configured alignment should not change. *Work-Around:* Developers can work-around this issue by setting the "textAlignment" and "verticalAlignment" properties after setting the "padding" property. *Cause:* The Java TiUIText.setTextPadding() method is resetting the field's gravity to CENTER_VERTICAL when it shouldn't. https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUIText.java#L233

Attachments

FileDateSize
After.png2017-06-21T03:29:21.000+000029925
Before.png2017-06-21T03:29:21.000+000030030
Screen Shot 2017-08-08 at 3.43.24 PM.png2017-08-08T22:45:02.000+000030420

Comments

  1. Michael Gangolf 2017-07-25

    PR * 6_2_x: https://github.com/appcelerator/titanium_mobile/pull/9247 * Master: https://github.com/appcelerator/titanium_mobile/pull/9248
  2. Samir Mohammed 2017-08-08

    [~michael] [~jquick] I am able to verify fix in SDK Version: 6.2.0.v20170808135421 and 7.0.0.v20170801134317 but on the newest build (7.0.0.v20170801134317) I am unable to verify. As you can see text is not correctly aligned: !Screen Shot 2017-08-08 at 3.43.24 PM.png|thumbnail! *Environment*
       Appcelerator Command-Line Interface, version 6.2.3-21
       Google nexus 6P 7.0.0 Emulator 
       Operating System Name: Mac OS X El Capitan
       Operating System Version: 10.11.6
       Node.js Version: 6.10.1
       Xcode: 8.2
       Appcelerator Studio: 4.9.0.201705302345
       
  3. Joshua Quick 2017-08-09

    [~smohammed], this bug is technically fixed. So, let's merge and close it anyways. What you're seeing now is a new issue that was accidentally introduced by this PR... https://github.com/appcelerator/titanium_mobile/pull/9114 In Titanium 7.0, TextField has been switched over to use Google's new/modern "TextInputLayout" which is capable of showing "hint" text above the input field (currently supported) and error/helper text below the input field (not supported yet). https://material.io/guidelines/components/text-fields.html#text-fields-layout Unfortunately, with the way it works now, the "height" property is applied to the TextInputLayout as a whole instead of the "EditText" input field in the middle like how it used to work. So, now there is a lot of empty space below the input field where the error/helper text is supposed to go. This also makes our "verticalAlign" property effectively useless. This is a breaking change, although to be fair, I would hope most developers would not hard code a height value (like the above sample project) since they run the risk of the font being clipped if too small or the field being ridiculously large compared to the font. (This is Google's expectation.) [~gmathews], I'm not quite sure what we should do about this. I have confirmed that setting the vertical layout parameter to MATCH_PARENT instead of WRAP_CONTENT resolves this issue when adding the EditText to the TextInputLayout.
       textInputLayout.addView(tv, new LinearLayout.LayoutParams(LinearLayout.LayoutParams.MATCH_PARENT, LinearLayout.LayoutParams.MATCH_PARENT));
       
    But the above will prevent the bottom error/helper text from being shown (we don't support this yet, but we may want to in the future). Plus doing the above goes against what Google recommends (possible future breakage). Applying a "weight" value to the EditText's layout parameters to have it fill the middle doesn't work either (I've tried). I'm now wondering if a better solution is to have the developer opt-in to the TextInputLayout behavior upon construction Ti.UI.createTextField() instead of always applying it. Hmm... any thoughts on this? *Side Note:* [~gmathews], there is a new "padding" bug on (master) that you can see when running this case's test project. The Java TiUIText.setPadding() is applying padding to the EditText (ie: the "tv" member variable) when it should be applied to the TextInputLayout instead. Doing this will make the field's bottom line line-up with the EditText above it and prevent them from overlapping if padding is set to zero.
  4. Gary Mathews 2017-08-09

    [~jquick] I think for the time being we should implement the workaround you found, setting the vertical layout parameter to MATCH_PARENT and create a ticket for this issue. Since this will maintain our expected behaviour until we figure out a better fix and we can discuss ideas in the new ticket.
  5. Gary Mathews 2017-08-28

    master: https://github.com/appcelerator/titanium_mobile/pull/9361
  6. Lokesh Choudhary 2017-11-13

    FR Passed. Waiting for merge to get enabled.
  7. Abir Mukherjee 2017-11-15

    Changes are seen in SDK 7.0.0.v20171114202841.

JSON Source