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-23501] iOS: Label.ellipsize should not return undefined #8086

Merged
merged 3 commits into from Jun 29, 2016

Conversation

AngelkPetkov
Copy link
Contributor

@hansemannn hansemannn added this to the 6.0.0 milestone Jun 24, 2016
if ([TiUtils intValue:value] == 1) {
[[self label] setLineBreakMode:NSLineBreakByTruncatingTail];
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cannot remove the old implementation, we need to throw at least a deprecation warning for boolean inputs.

@m1ga
Copy link
Contributor

m1ga commented Jun 28, 2016

I've added the android part and set the default value to false because you can turn it off that way (and its off when you just create a label). So the return value is a boolean in my case. Is there a way to turn it off on iOS, too?

if ([TiUtils intValue:value] == 1) {
[[self label] setLineBreakMode:NSLineBreakByTruncatingTail];
[[self label] setLineBreakMode:NSLineBreakByCharWrapping];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of this, please check if the input is a bool and leave the if-block as it is, if so.

@hansemannn
Copy link
Collaborator

@AngelkPetkov please cherry-pick his Android-changes into this PR and I will FT it.

@AngelkPetkov
Copy link
Contributor Author

Updated Boolean check and added #8094.

Thank you @m1ga !

@AngelkPetkov AngelkPetkov force-pushed the TIMOB-23501 branch 3 times, most recently from 18d8c00 to 2cbf003 Compare June 29, 2016 21:38
AngelkPetkov and others added 2 commits June 29, 2016 14:40
# Conflicts:
#	android/modules/ui/src/java/ti/modules/titanium/ui/LabelProxy.java
@hansemannn
Copy link
Collaborator

hansemannn commented Jun 29, 2016

There is an issue here @AngelkPetkov @m1ga:

var label = Ti.UI.createLabel();

Ti.API.warn(label.getEllipsize()); // Should be "4" (Integer for Ti.UI.TEXT_ELLIPSIZE_TRUNCATE_END - default)
Ti.API.warn(label.getEllipsize() == Ti.UI.TEXT_ELLIPSIZE_TRUNCATE_END); // Should be "true"
Ti.API.warn(label.getEllipsize() == Ti.UI.TEXT_ELLIPSIZE_TRUNCATE_MIDDLE); // Should be "false"

label.setEllipsize(Ti.UI.TEXT_ELLIPSIZE_TRUNCATE_MIDDLE);

Ti.API.warn(label.getEllipsize()); // Should be "5" (Integer for Ti.UI.TEXT_ELLIPSIZE_TRUNCATE_MIDDLE)
Ti.API.warn(label.getEllipsize() == Ti.UI.TEXT_ELLIPSIZE_TRUNCATE_MIDDLE); // Should be "true"
Ti.API.warn(label.getEllipsize() == Ti.UI.TEXT_ELLIPSIZE_TRUNCATE_END); // Should be "false"

As the constants only represent a name for a number, the above logs output numbers. On iOS, it is 4 and 5, on Android it is 2 and 1. I think, as long as the user checks agains the constants and not against the numbers, it's fine. We cannot ensure that all constants across all platforms represent the same number, so this is the way to go. Do you guys agree?

@@ -1243,6 +1243,30 @@ properties:
permission: read-only
platforms: [iphone, ipad]

- name: TEXT_ELLIPSIZE_TRUNCATE_WORD_WRAP
summary: Add ellipses at word boundaries, unless the word itself doesnt fit on a single line
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • doesnt -> doesn't
  • Missing period at the end of the sentence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah i had it as doesn't originally however node validate doesn't like ' as its a non ASCII character, so i removed it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the correct sign there as we do in related docs. Search UI.yml for other usages and copy it from there.

@hansemannn
Copy link
Collaborator

FT passed, please address the documentation comments before merging.

[TIMOB-23501] iOS: fix IF
@hansemannn
Copy link
Collaborator

CR + FT passed, PR approved.

@hansemannn hansemannn merged commit 54a11a4 into tidev:master Jun 29, 2016
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.

None yet

3 participants