RichPath icon indicating copy to clipboard operation
RichPath copied to clipboard

RichPath for Kotlin

Open ivybae opened this issue 6 years ago • 20 comments

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?

ivybae avatar Sep 18 '19 16:09 ivybae

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?

tarek360 avatar Sep 19 '19 14:09 tarek360

@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?

ivybae avatar Sep 19 '19 15:09 ivybae

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!

tarek360 avatar Sep 19 '19 15:09 tarek360

@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!

ivybae avatar Sep 20 '19 08:09 ivybae

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

tarek360 avatar Sep 20 '19 10:09 tarek360

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

ivybae avatar Sep 20 '19 10:09 ivybae

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

vipafattal avatar Sep 20 '19 10:09 vipafattal

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

ivybae avatar Sep 20 '19 11:09 ivybae

Sure, I'd be glad to.

vipafattal avatar Sep 20 '19 11:09 vipafattal

@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?

ivybae avatar Sep 20 '19 12:09 ivybae

Great idea! Ok.

vipafattal avatar Sep 20 '19 12:09 vipafattal

@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! :)

ivybae avatar Sep 24 '19 16:09 ivybae

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!

tarek360 avatar Sep 24 '19 17:09 tarek360

@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?

ivybae avatar Sep 25 '19 13:09 ivybae

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.

tarek360 avatar Oct 03 '19 04:10 tarek360

Hi, @tarek360 that's ok. I understand. I created a PR minutes ago (https://github.com/tarek360/RichPath/pull/55). I'm looking forward to your advice. Thanks!

ivybae avatar Oct 03 '19 09:10 ivybae

A new PR: https://github.com/tarek360/RichPath/pull/60

tarek360 avatar Feb 01 '20 09:02 tarek360

@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?

ivybae avatar Feb 01 '20 15:02 ivybae

@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

tarek360 avatar Feb 02 '20 08:02 tarek360

@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 avatar Feb 19 '20 14:02 ivybae