agoda-com / Kakao

This repo is no longer supported. Please visit a https://github.com/KakaoCup/Kakao
Apache License 2.0
1.11k stars 102 forks source link

Add KSwitch #168

Closed sebastienrouif closed 4 years ago

sebastienrouif commented 4 years ago

Android Switch definition : A Switch is a two-state toggle switch widget that can select between two options. The user may drag the "thumb" back and forth to choose the selected option, or simply tap to toggle as if it were a checkbox.

Adding a KSwitch would allow developers to test both click and swipe gesture and make sure they implemented the OnCheckedChangeListener

Unlimity commented 4 years ago

Hi! Thanks for submitting the issue. Can you please clarify what is the distinct difference and benefit to have KSwitch when we have KCheckBox? Both Switch and CheckBox extend CompoundButton and implement Checkable interface. This is already supported within Kakao with KCheckBox.

sebastienrouif commented 4 years ago

I guess this is more of a different mindset on how to approach the problem. Accessing the View directly and changing it's checked state is not correct to me. The users can't do that through the UI. They can only do press/longPress/swipe/pinch ... A concrete example is to call .setChecked(false) on a disabled CheckBox and it will still change it's state, which should not be allowed. To me KCheckBox should not implement CheckableActions. However Switch supports gesture and creating methods to perform those gestures would be helpful

Unlimity commented 4 years ago

Seems like a strong point. @Vacxe what do you think? @sebastienrouif are you willing to contribute?

sebastienrouif commented 4 years ago

@Unlimity , yes. I don't want to remove CheckableActions as this is a strong point of view and some users of this library might be unhappy with this change. Most of the work is already done, it would be about testing, documenting and polishing it (I'm not sure how we should support RTL):

class KSwitch : KBaseView<KSwitch>,
                TextViewAssertions,
                CheckableAssertions,
                SwipeableActions {

    constructor(function: ViewBuilder.() -> Unit) : super(function)
    constructor(parent: Matcher<View>, function: ViewBuilder.() -> Unit) : super(parent, function)
    constructor(parent: DataInteraction, function: ViewBuilder.() -> Unit) : super(parent, function)
}
Unlimity commented 4 years ago

I'm not sure there is need to support RTL in that case, since there is swipeLeft and swipeRight available. Usually, the tests are being executed in a single environment, and if there is need to test RTL, the tests are usually implemented independently.