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

[TIMOB-24479] Android: fix TiUILabel maxLines not working #8884

Merged
merged 1 commit into from Mar 17, 2017

Conversation

frankieandone
Copy link
Contributor

@frankieandone frankieandone commented Mar 13, 2017

Test Case:
Expect the long string to be in one line meaning no word wraps.

var win = Ti.UI.createWindow({
	theme: "Theme.AppCompat.Fullscreen",
	backgroundColor: '#fff'
});
var longString = "Welcome to the Appcelerator Platform! The Appcelerator Platform extends Appcelerator's Titanium platform to help you manage the entire lifecycle of the application with debugging, testing, deployment, crash monitoring and analytic data collection.";
var bigLabel = Ti.UI.createLabel({
	text: longString,
	color: '#4d4d4d',
	width: "90%",
	top: 0,
	maxLines: 1
});
win.add(bigLabel);
win.open();

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

Copy link
Contributor

@antw12 antw12 left a comment

Choose a reason for hiding this comment

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

CR & FT PASS

@ssjsamir ssjsamir self-assigned this Mar 13, 2017
@frankieandone frankieandone force-pushed the timob-24479 branch 2 times, most recently from 259dd2f to 1457331 Compare March 14, 2017 02:02
@@ -329,7 +334,7 @@ public void processProperties(KrollDict d)
TiUIHelper.linkifyIfEnabled(tv, d.get(TiC.PROPERTY_AUTO_LINK));
tv.invalidate();
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you do the backport manually? I don't see this line in the backport, so I was wondering. Try to cherry-pick all changes for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I did the backport manually which is linked here.

@garymathews
Copy link
Contributor

garymathews commented Mar 14, 2017

@fmerzadyan Do we know what broke this in 6.0.0? I'm curious why we now need to refresh maxLines.

@garymathews garymathews self-requested a review March 14, 2017 16:09
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.

@fmerzadyan I think the problem is with wordWrap defaulting to true. So when maxLines of 1 is specified, the label enables wordWrap after setting maxLines, causing the line to wrap.

Try moving maxLines so it is processed after wordWrap

if (d.containsKey(TiC.PROPERTY_WORD_WRAP)) {
	wordWrap = TiConvert.toBoolean(d, TiC.PROPERTY_WORD_WRAP, true);
	tv.setSingleLine(!wordWrap);
}
if (d.containsKey(TiC.PROPERTY_MAX_LINES)) {
	tv.setMaxLines(TiConvert.toInt(d, TiC.PROPERTY_MAX_LINES));
}

@frankieandone
Copy link
Contributor Author

frankieandone commented Mar 14, 2017

@garymathews, the problem is that setSingleLine resets the maxLines property and setSingleLine method is used in multiple places in TiUILabel so the fix was to address those situations now rather than have this problem possibly come up in the future.

@garymathews
Copy link
Contributor

garymathews commented Mar 14, 2017

@fmerzadyan I think it would be best to just move maxLines to be set after all of the properties that use setSingleLine and leave a comment to explain why maxLines should be processed last. This will fix all scenarios without requiring a method to be called after the use of setSingleLine.

@frankieandone
Copy link
Contributor Author

@garymathews, No problem 👍

@antw12
Copy link
Contributor

antw12 commented Mar 14, 2017

CR & FT PASS with new changes

@ssjsamir
Copy link
Contributor

FR Passed was able to set a long string to 1 line.

Steps taken to test

  • Copied the test case in the description
  • Pasted the code into a titanium project in appcelerator studio
  • Ran the program on device
  • Saw that string was set to one line as part of the string was missing

Environment
Appcelerator Command-Line Interface, version 6.1.0
Nexus 6P (7.1.1)
Operating System Name: Mac OS X El Capitan
Operating System Version: 10.11.6
Node.js Version: 4.6.0
npm: 4.2.8
Xcode: 8.2
Appcelerator Studio: 4.8.1.201612050850

@garymathews garymathews merged commit ee0e9e0 into tidev:master Mar 17, 2017
@frankieandone frankieandone deleted the timob-24479 branch April 5, 2017 17:33
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

5 participants