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

Add extension functions to initialize Loupe and set listeners #15

Merged
merged 2 commits into from
Jul 16, 2020
Merged

Add extension functions to initialize Loupe and set listeners #15

merged 2 commits into from
Jul 16, 2020

Conversation

adrianolc
Copy link
Contributor

Just a nice to have some extension functions to help initialize the lib and to set the listeners more in a Kotlin style.

val loupe = startLoupe(image, container) {
    useFlingToDismissGesture = !Pref.useSharedElements
    maxZoom = Pref.maxZoom
    flingAnimationDuration = Pref.flingAnimationDuration
    scaleAnimationDuration = Pref.scaleAnimationDuration
    overScaleAnimationDuration = Pref.overScaleAnimationDuration
    overScrollAnimationDuration = Pref.overScrollAnimationDuration
    dismissAnimationDuration = Pref.dismissAnimationDuration
    restoreAnimationDuration = Pref.restoreAnimationDuration
    viewDragFriction = Pref.viewDragFriction

    setOnViewTranslateListener(
        onStart = {
            hideToolbar()
        },
        onRestore = {
            showToolbar()
        },
        onDismiss = {
            finishAfterTransition()
        }
    )

    setOnScaleChangedListener { scaleFactor, focusX, focusY ->

    }
}

@igreenwood
Copy link
Owner

igreenwood commented Jul 14, 2020

@adrianolc
Thank you for your nice proposal 😄
It's great to be able to write in Kotlin's style.
But I think it might be better if it wasn't an extension like View or Activity. Probably because most users will only use Loupe on a limited number of screens. 🤔

My suggestion:

  • Create startLoupe() function in the companion object of Loupe ( I want to change the name startXXX to createXXX because "start" sounds like an asynchronous processing function. 🙏 )
  • Add the setOnViewTranslateListener() and setOnScaleChangedListener() that you created into Loupe class.

Looks like this↓

class Loupe(imageView: ImageView, container: ViewGroup) : View.OnTouchListener,
    View.OnLayoutChangeListener {

    companion object {

        fun create(
            imageView: ImageView,
            container: ViewGroup,
            config: Loupe.() -> Unit = {}
        ): Loupe {
            return Loupe(
                imageView,
                container
            ).apply(config)
        }
    }
    
    ......

    fun setOnViewTranslateListener(
        onStart: (view: ImageView) -> Unit = {},
        onViewTranslate: (view: ImageView, amount: Float) -> Unit = { _, _ -> },
        onRestore: (view: ImageView) -> Unit = {},
        onDismiss: (view: ImageView) -> Unit = {}
    ) {
        this.onViewTranslateListener = object : OnViewTranslateListener {
            override fun onDismiss(view: ImageView) {
                onDismiss(view)
            }

            override fun onRestore(view: ImageView) {
                onRestore(view)
            }

            override fun onStart(view: ImageView) {
                onStart(view)
            }

            override fun onViewTranslate(view: ImageView, amount: Float) {
                onViewTranslate(view, amount)
            }
        }
    }

    fun setOnScaleChangedListener(onScaleChangedListener: (scaleFactor: Float, focusX: Float, focusY: Float) -> Unit) {
        this.onScaleChangedListener = object : OnScaleChangedListener {
            override fun onScaleChange(scaleFactor: Float, focusX: Float, focusY: Float) {
                onScaleChangedListener(scaleFactor, focusX, focusY)
            }
        }
    }
}

How about that? 🙏

@adrianolc
Copy link
Contributor Author

Hi @igreenwood, thanks for your comment.

The idea of having extensions function is to not pollute the class if helper methods.

But I agree on the startLoupe method, create is a better name and also I don't see any problem with it being a method on the companion object.

What do you think? Bring the startLoupe to the companion object and rename it to only create, but let the setters as an extension function as they're only helpers and not something to include in the class.

@igreenwood
Copy link
Owner

Hi @adrianolc, I'm glad you agree with me about Loupe.create().
The extension itself is very useful and good. However, different developers have different usage preferences, so I'd like to keep the option of not using extensions.
As for the extensions, I think we could get both of our hopes up by releasing them as a separate module, what do you think? 😉
If it's OK, then this PR could be merged with only a change to Loupe.create() and I will begin to create a module for extensions 💪

@adrianolc
Copy link
Contributor Author

All good!! I agree with you!

I can do the changes :)

@adrianolc
Copy link
Contributor Author

Changed, so now only have the create function

val loupe = Loupe.create(image, container) {
    useFlingToDismissGesture = !Pref.useSharedElements
    maxZoom = Pref.maxZoom
    flingAnimationDuration = Pref.flingAnimationDuration
    scaleAnimationDuration = Pref.scaleAnimationDuration
    overScaleAnimationDuration = Pref.overScaleAnimationDuration
    overScrollAnimationDuration = Pref.overScrollAnimationDuration
    dismissAnimationDuration = Pref.dismissAnimationDuration
    restoreAnimationDuration = Pref.restoreAnimationDuration
    viewDragFriction = Pref.viewDragFriction
}

@igreenwood igreenwood merged commit 83a73ae into igreenwood:master Jul 16, 2020
@igreenwood
Copy link
Owner

@adrianolc Thank youuuuuuu!!! I will release loupe: 1.0.4 and loupe-ktx:1.0.0 ASAP 💪

@adrianolc
Copy link
Contributor Author

@igreenwood Niiiice!!!

I'm looking forward to remove the fork from our App and use the maven to implement the library!!

@igreenwood
Copy link
Owner

@adrianolc I have released com.igreenwood.loupe:loupe:1.1.0 & com.igreenwood.loupe:extensions:1.0.0 😎

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

2 participants