[TIMOB-16512] Android: added left and right padding to TextField #5492
Conversation
|
Please add documentation and a test case. |
|
will update |
| tv.setPadding(TiConvert.toInt(d, TiC.PROPERTY_PADDING_LEFT), 0, 0, 0); | ||
| } | ||
| if (d.containsKey(TiC.PROPERTY_PADDING_RIGHT)) { | ||
| tv.setPadding(0, 0, TiConvert.toInt(d, TiC.PROPERTY_PADDING_RIGHT), 0); |
There was a problem hiding this comment.
This is incorrect. If I have padding left and right properties set, the padding right will override the padding left. You need to keep track of these paddings and apply them properly.
|
The implementation is incorrect. Please address comments. |
|
@bijupmb There has been no update on this PR for over a week now. We should follow up proactively on these PR comments. |
|
Already modified |
|
@hieupham007 - Please review this PR. |
| @@ -262,7 +290,7 @@ public void propertyChanged(String key, Object oldValue, Object newValue, KrollP | |||
| TiUIHelper.styleText(tv, (HashMap) newValue); | |||
| } else if (key.equals(TiC.PROPERTY_AUTO_LINK)) { | |||
| TiUIHelper.linkifyIfEnabled(tv, newValue); | |||
| } else { | |||
| } else if (!setTextPadding(proxy.getProperties())){ | |||
There was a problem hiding this comment.
This implementation is inefficient. You don't need to call it every time a property is changed.. You only need to call it when paddingRight and Left is called.
|
Please extend this to implement padding top and bottom as well. Also update documentation and test case to include them. |
|
Code reviewed. Please address comments |
|
start working on this |
|
Done |
ingo
changed the title
ingo
changed the title
|
@hieupham007 Can you review the latest code please? |
| platforms: [iphone, ipad] | ||
| platforms: [iphone, ipad, android] | ||
|
|
||
| - name: paddingBottom |
There was a problem hiding this comment.
Indentation 2 spaces left
|
I really like this PR, although the CLA is not signed and there are merge conflicts with latest SDK. @ashcoding can you investigate what we could to to support this? It would be great for parity-reasons, here are the current docs. |
|
@bijupmb hope it's fine that I updated your PR to be merged in the latest branch again! |
|
Closing this PR in favor of #7908 to incorporate with the suggested changes. Thanks guys! |
https://jira.appcelerator.org/browse/TIMOB-16512