Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

RichPath for Kotlin #53

Open
ivybae opened this issue Sep 18, 2019 · 20 comments
Open

RichPath for Kotlin #53

ivybae opened this issue Sep 18, 2019 · 20 comments

Comments

@ivybae
Copy link
Collaborator

ivybae commented Sep 18, 2019

Hi, @tarek360 :)
I'm very impressed your work.
I think RichPath is the best sample for learning VectorDrawable animation.
I really appreciate it.
Do you by any chance make kotlin version?

@tarek360
Copy link
Owner

Hi :)

I'm happy to hear that :)

I don't think I have time to convert it to Kotlin, but I'll be more than happy to see a contribution on this.

In the meantime, do you have any compatibility problems with Kotlin?

@ivybae
Copy link
Collaborator Author

ivybae commented Sep 19, 2019

@tarek360
Thanks for your reply :)
I didn't have any compatibility issues.
I want to participate converting to Kotlin. That's why I just asked.
So, I wonder your opinion. How should I contribute to RichPath? Should I make a Kotlin branch?

@tarek360
Copy link
Owner

Awesome!

As you see in the root project, RichPath has two modules:

  1. animator
  2. richpath

I would say you can start by the smaller one (you can break it down as you want)
Sure, you can check out a new branch from develop and create a PR when you think it's ready, so I can review it. Don't hesitate to ask any question if you found something not clear.

Thank you!

@ivybae
Copy link
Collaborator Author

ivybae commented Sep 20, 2019

@tarek360
Thanks for your kindness!

Then, where I write a kotlin code?
For example, I'm gonna create module like 'animator-kotlin' or 'richpath-kotlin'.
I wanna your opinion.

Have a good day!

@tarek360
Copy link
Owner

we have /richpath/src/main/java and /animator/src/main/java
so you need to create a new source directory for kotlin for each module like the following:
/richpath/src/main/kotlin
/animator/src/main/kotlin

@ivybae
Copy link
Collaborator Author

ivybae commented Sep 20, 2019

@tarek360
Oh, I understand.
I'm gonna fork your repository, then start from animator.
Thank you.

@vipafattal
Copy link

@step4me Sounds a good idea, would you like some help?

@ivybae
Copy link
Collaborator Author

ivybae commented Sep 20, 2019

@vipafattal
Thanks! I'm just about to start.
Do you wanna work together, by any chance?

@vipafattal
Copy link

Sure, I'd be glad to.

@ivybae
Copy link
Collaborator Author

ivybae commented Sep 20, 2019

@vipafattal
Good! :)
I'm gonna request PR in few days about some part of animator module.
After that, we would be better collaborate with.
What do you think about it?

@vipafattal
Copy link

Great idea! Ok.

@ivybae
Copy link
Collaborator Author

ivybae commented Sep 24, 2019

@tarek360
Hi, I wanna share my progress and need your advices.
I added 'kotlin source set' in animator module and converted to Kotlin classes.

  1. package name : > /animator/src/main/kotlin/{richpathanimator}/AnimationBuilder.kt
    I added 'richpathanimator' package. Because {com.richpathanimator} package cannot have duplicate classes.
    What do you think I should do?

  2. AnimationBuilder
    I did convert like this.

public AnimationBuilder durationSet(long duration) { richPathAnimator.duration(duration); return this; }

fun durationSet(duration: Long) = apply { richPathAnimator.duration = duration }

Like above, I convert 'AnimationBuilder return type' using apply inline function.
Then, I thought how about add 'build function' like this:
fun build(block: AnimationBuilder.() -> Unit) = apply(block)

Have a good day! :)

@tarek360
Copy link
Owner

Hi :)
Great progress 👍
You shouldn't have duplicate classes, You should remove any Java class you converted to Kotlin, or what duplicates you have?
Regarding the second point, please let me check later when I can open my laptop!

@ivybae
Copy link
Collaborator Author

ivybae commented Sep 25, 2019

@tarek360
Oh, I thought we have two source sets, java and kotlin. So the first one is resolved. I'm gonna remove java classes.

And, Is it better to discuss my work so far through PR with regard to the second point?

@tarek360
Copy link
Owner

tarek360 commented Oct 3, 2019

Hi @step4me sorry for the late reply,
Yes, if you already converted some classes don't hesitate to create a PR then we can discuss easily on it.

@ivybae
Copy link
Collaborator Author

ivybae commented Oct 3, 2019

Hi, @tarek360 that's ok. I understand.
I created a PR minutes ago (#55).
I'm looking forward to your advice.
Thanks!

@tarek360
Copy link
Owner

tarek360 commented Feb 1, 2020

A new PR: #60

@ivybae
Copy link
Collaborator Author

ivybae commented Feb 1, 2020

@tarek360
I saw PR #60. I appreciate your commits about #58, #59 and java/kotlin compatibility.

I am going to convert java to kotlin in richpath module.

Also, I found out unexpected result.
icCommandRichPathView dosen't repeat infinite. Do you have any ideas about it?

@tarek360
Copy link
Owner

tarek360 commented Feb 2, 2020

@step4me cool! go on 👍
I fixed it after many hours trying to find where is the problem :D

actually repeatMode and repeatCount were private properties but you didn't make them private in Kotlin and you called them by mistake instead of the methods.

https://github.com/tarek360/RichPath/pull/55/files#diff-2503c785024291f403b109b2ad1afd6aR57-R60

we should call:

animationBuilder.repeatMode(repeatMode)
animationBuilder.repeatCount(repeatCount)

instead of

animationBuilder.repeatMode = repeatMode
animationBuilder.repeatCount = repeatCount

@ivybae
Copy link
Collaborator Author

ivybae commented Feb 19, 2020

@tarek360
Hi, I saw your last commit. I am sorry, it was my mistake.
I really appreciate your review and modifications.

I converted some classes java to kotlin in richpath module.
Please review and let me know for these changes on #60.

@ivybae ivybae mentioned this issue Jun 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants