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

fix(android): textfield padding #13513

Merged
merged 6 commits into from
Sep 14, 2024
Merged

Conversation

hansemannn
Copy link
Collaborator

cc @m1ga, you can push into the branch :)

@m1ga
Copy link
Contributor

m1ga commented Jul 20, 2022

Added the dpi fix!

Still have to test the other borderStyles but this fix will already work for your app when you just set padding: { left: 20 } for both platforms in your profile page.

@hansemannn
Copy link
Collaborator Author

Yep, thats great! Still, we will likely have to wait for the 12.0.0 release, since many devs like us used the workaround with different padding for now, so those apps would break in silence. No issue though for the next major, as it can be listed as one of the behavioral / breaking changes.

@m1ga
Copy link
Contributor

m1ga commented Jul 21, 2022

add the option to use tv.padding = undefined to reset to the default values again.

Example

const win = Ti.UI.createWindow({});
const vLeft = Ti.UI.createView({top: 0, layout:"vertical", height: Ti.UI.SIZE,left:0,width: "45%"});
const vRight = Ti.UI.createView({top: 0, layout:"vertical", height: Ti.UI.SIZE,right:0,width: "45%"});
const lbl = Ti.UI.createLabel({bottom: 50});
const btn1 = Ti.UI.createButton({title:"padding", bottom: 0, left:0});
const btn2 = Ti.UI.createButton({title:"text", bottom: 0, right:0});
const btn3 = Ti.UI.createButton({title:"height", bottom: 0});

var objs = [];
var padding = 0;
var hasText = false;
btn1.addEventListener("click",function(){
	for(var i=0;i<objs.length;++i){
		if (padding == 0) objs[i].padding = {top:10,left:10,right:10,bottom:10}
		if (padding == 1) objs[i].padding = {top:0,left:10,right:10,bottom:0}
		if (padding == 2) objs[i].padding = {top:10,left:0,right:0,bottom:10}
		if (padding == 3) objs[i].padding = {top:0,left:0,right:0,bottom:0}
		if (padding == 4) objs[i].padding = undefined;
	}
	lbl.text = objs[0].padding;
	padding++;
	if (padding>=5) {padding = 0;}
})

btn2.addEventListener("click",function(){
	for(var i=0;i<objs.length;++i){
		if (hasText){objs[i].value = "";} else {objs[i].value = "test value";}
	}
	hasText = !hasText;
})
btn3.addEventListener("click",function(){
	for(var i=0;i<objs.length;++i){
		objs[i].height = (objs[i].height==40)?Ti.UI.SIZE:40;
	}
})

function addTf(options, view){
	var tf = Ti.UI.createTextField({ returnKeyType:Titanium.UI.RETURNKEY_DONE, hintText:"test", width: "90%", bottom: 10});
	tf.applyProperties(options);
	objs.push(tf);
	view.add(tf);
}

addTf({}, vLeft);
addTf({borderStyle : Titanium.UI.INPUT_BORDERSTYLE_BEZEL}, vLeft);
addTf({borderStyle : Titanium.UI.INPUT_BORDERSTYLE_LINE}, vLeft);
addTf({borderStyle : Titanium.UI.INPUT_BORDERSTYLE_NONE}, vLeft);
addTf({borderStyle : Titanium.UI.INPUT_BORDERSTYLE_ROUNDED}, vLeft);
addTf({borderStyle : Titanium.UI.INPUT_BORDERSTYLE_UNDERLINED}, vLeft);
addTf({borderStyle : Titanium.UI.INPUT_BORDERSTYLE_FILLED}, vLeft);

addTf({hintType: Titanium.UI.HINT_TYPE_ANIMATED}, vRight);
addTf({hintType: Titanium.UI.HINT_TYPE_ANIMATED,borderStyle : Titanium.UI.INPUT_BORDERSTYLE_BEZEL}, vRight);
addTf({hintType: Titanium.UI.HINT_TYPE_ANIMATED,borderStyle : Titanium.UI.INPUT_BORDERSTYLE_LINE}, vRight);
addTf({hintType: Titanium.UI.HINT_TYPE_ANIMATED,borderStyle : Titanium.UI.INPUT_BORDERSTYLE_NONE}, vRight);
addTf({hintType: Titanium.UI.HINT_TYPE_ANIMATED,borderStyle : Titanium.UI.INPUT_BORDERSTYLE_ROUNDED}, vRight);
addTf({hintType: Titanium.UI.HINT_TYPE_ANIMATED,borderStyle : Titanium.UI.INPUT_BORDERSTYLE_UNDERLINED}, vRight);
addTf({hintType: Titanium.UI.HINT_TYPE_ANIMATED,borderStyle : Titanium.UI.INPUT_BORDERSTYLE_FILLED}, vRight);
lbl.text = objs[0].padding;
win.add([vLeft,vRight,btn1,btn2,lbl,btn3]);
win.open();

Screenshot_20220721-143426

This is comparing this PR (left) and 11.0.0.GA (right) with height: Ti.UI.SIZE and padding 0 for all corners:
Screenshot_20220721-143552 Screenshot_20220721-143647

current GA doesn't remove the padding, with this PR you to have more control of the output of the Textfield and it will use Ti.UI.SIZE correctly.

Fixed height (padding 0): new vs old
Screenshot_20220721-144331 Screenshot_20220721-144208

so it's possible to lower the height of the Textfields now.

Still doing some more tests 😉

@hansemannn
Copy link
Collaborator Author

@m1ga Can you approve the changes? Then I can merge it

@m1ga
Copy link
Contributor

m1ga commented Sep 14, 2024

it's my old PR #13279 so I'm not sure if I should approve it 😄 It was reverted because of a logicalDensityFactor issue. We can revert this and I'll make a new PR we can test more. issue. This is fixed.

It's still working fine:

  • if you don't use any padding it will look the same
  • if you use padding you might have to adjust it in your app as it will now work "correctly" (images in fix(android): textfield padding #13279)

@m1ga m1ga self-requested a review September 14, 2024 16:04
Copy link
Contributor

@m1ga m1ga left a comment

Choose a reason for hiding this comment

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

LGTM (biased as I did the change 😄 )

@hansemannn
Copy link
Collaborator Author

I know, but we need the formal approval for a PR merge :)

@hansemannn hansemannn merged commit fac2c4b into master Sep 14, 2024
5 checks passed
@hansemannn hansemannn deleted the revert-13512-revert-13279-textfield branch September 14, 2024 16:06
hansemannn added a commit that referenced this pull request Sep 16, 2024
* Revert "Revert "fix(android): textfield padding (#13279)" (#13512)"

This reverts commit 918388a.

* fix(android): fix input value

* reset padding

---------

Co-authored-by: m1ga <[email protected]>
Co-authored-by: Michael Gangolf <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants