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

Kotlin migration #19

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Kotlin migration #19

wants to merge 18 commits into from

Conversation

elevenetc
Copy link
Owner

@elevenetc elevenetc commented Feb 15, 2021

  • Converted to Kotlin
    • ScaleValue and Position converted into data classes
    • Getters and setters replaced with fields where possible
    • Static utility methods replaced extensions functions where possible
    • Java collections replaced with Kotlin ones
    • Functional interfaces replaced with lambdas
    • String concat replaced with kotlin templates
  • Improved package structure
  • Simplified core entities hierarchy
  • Removed excessive invalidate calls (text reveal animations)
  • Renamed generic Debug to TextSurfaceDebug to avoid spoiling namespace
  • Renamed TextBuilder > Text.Builder
  • Added @CallSuper to TextEffect.onStart
  • Added @CallSuper to SurfaceAnimation.cancel
  • Added support of ScrollView Fixes It doesn't work if it's in ScrollView #13

typeface = Typeface.createFromAsset(assetManager, "fonts/Roboto-Black.ttf")
}

val textDaai = Text.Builder("Daai")
Copy link

@ptmt ptmt Feb 15, 2021

Choose a reason for hiding this comment

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

Seems like using Java API from Kotlin, feels a bit verbose. I would expect something more idiomatic to Kotlin (Data classes, type-safe builders, or just apply with mutable fields as you do earlier?) Maybe any reason for set..?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah, I agree. I actually tried to get rid of TextBuilder, but there two problems:

  1. Making of Position instance has it's own requirements and there're 3 most frequent cases and they are hidden in setters:
val position1 = Position(align, alignText = alignText)
val position2 = Position(align)
val position3 = Position(point = PointF(px, py))
  1. For most cases scale and scale pivot needs to be set correctly and it's done in final build call:
val text = Text(text, position, padding, paint)
text.scaleX = scale
text.scaleY = scale
text.setScalePivot(scalePivot, scalePivot)

So for most cases Builder hides these requirements inside setters and final build function. For custom (and probably very rare cases with weird position and maybe inverted or negative scale) cases developer can use Position and Text constructors without Builder.

But it might be that these requirements can be hidden inside type-safe builder. Need to think about better api.

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.

It doesn't work if it's in ScrollView
2 participants