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-23500] Android: default value for ellipsize #8094

Closed
wants to merge 3 commits into from
Closed

[TIMOB-23500] Android: default value for ellipsize #8094

wants to merge 3 commits into from

Conversation

m1ga
Copy link
Contributor

@m1ga m1ga commented Jun 28, 2016

@build
Copy link
Contributor

build commented Jun 28, 2016

Can one of the admins verify this patch?

@AngelkPetkov
Copy link
Contributor

Hello, thank you for the contribution! However the value for the ellipsize should not actually be a boolean, but it should return the constant. An Android fix is in progress #8086 for parity reasons.

@m1ga
Copy link
Contributor Author

m1ga commented Jun 28, 2016

I've added a comment in your PR :) Sorry for crossposting

@AngelkPetkov
Copy link
Contributor

FT Approved!

@@ -135,6 +135,7 @@ public boolean onTouchEvent(MotionEvent event) {
tv.setKeyListener(null);
tv.setFocusable(false);
tv.setSingleLine(false);
tv.setEllipsize(ellipsize);
Copy link
Collaborator

Choose a reason for hiding this comment

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

How does that effect current labels that do not have any ellipsize property defined? We should try to ensure that nothing breaks after this change (although 6.0 would be the right time to do this).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it will just be a "good" change:
ell

it will just affect the fields where you have a fixed width (no wordWrap) and the text would be out of the label anyway. That's mostly the point where you are searching on how to switch on ellipsize :)
But since Android can switch of the ellipsizing it might have a different default value. I would prefer truncateAt.END because it will make the label look the same on both platforms from the beginning. You decide ;)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Like it, thanks for that great example!

@hansemannn
Copy link
Collaborator

Cherry-picked into #8086 to be able to test both together. Thank you @m1ga, good work! 🚀

@hansemannn hansemannn closed this Jun 29, 2016
@m1ga m1ga deleted the ellipsize branch August 9, 2016 16:23
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

4 participants