Skip to content
This repository has been archived by the owner on May 29, 2023. It is now read-only.

Conversation

amuthansakthivel
Copy link
Contributor

@amuthansakthivel amuthansakthivel commented Jan 1, 2023

Added AppiumSetValueOptions to hide keyboard if needed after setting value to textbox.

It is a painful process to hide keyboard after every setValue. Hiding keyboard is not needed always. It is needed only if the screen size is small or element is at bottom of the screen.

So we want to leave the choice to users.

$(element).setValue(withText("abcd123").hideKeyboard());
$(element).setValue(withText("abcd123"));
$(element).setValue("abcd123");

@BorisOsipov

Checklist

  • Checkstyle and unit tests are passed locally with my changes by running gradlew check command
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@amuthansakthivel amuthansakthivel changed the title added AppiumSetValueOptions to hide keyboard after entering a value u… Added AppiumSetValueOptions to hide keyboard Jan 1, 2023
@amuthansakthivel
Copy link
Contributor Author

@asolntsev @BorisOsipov - This PR is now ready for review. Thanks

@sonarcloud
Copy link

sonarcloud bot commented Jan 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@asolntsev
Copy link
Member

@amuthansakthivel The change is good technically, but I am thinking usability...
I mean, instead if writing

$.setValue(withText("abcd").hideKeyboard());

Isn't it simpler to just write setValue and hideKeyboard separately?

$.setValue("abcd").hideKeyboard();

The only thing we need to achieve that is make method $ return something like SelenideAppiumElement instead of SelenideElement. Probably we should do it anyway.
Then users will need to import com.codeborne.selenide.appium.SelenideAppium.$ instead of com.codeborne.selenide.Selenide.$. Seems easy.

@amuthansakthivel
Copy link
Contributor Author

amuthansakthivel commented Jan 30, 2023

@amuthansakthivel The change is good technically, but I am thinking usability... I mean, instead if writing

$.setValue(withText("abcd").hideKeyboard());

Isn't it simpler to just write setValue and hideKeyboard separately?

$.setValue("abcd").hideKeyboard();

The only thing we need to achieve that is make method $ return something like SelenideAppiumElement instead of SelenideElement. Probably we should do it anyway. Then users will need to import com.codeborne.selenide.appium.SelenideAppium.$ instead of com.codeborne.selenide.Selenide.$. Seems easy.

$.setValue("abcd").hideKeyboard() looks very readable.

But @asolntsev, when are we planning to implement that class? This SelenideAppiumElement class should also handle https://github.com/selenide/selenide-appium/issues/106
selenide/selenide#2300
selenide/selenide#2298

@asolntsev asolntsev self-assigned this Jan 31, 2023
@asolntsev
Copy link
Member

@amuthansakthivel Please take a look: I added method $.hideKeyboard() in PR #131

@amuthansakthivel
Copy link
Contributor Author

@asolntsev - i am closing this PR then

@amuthansakthivel amuthansakthivel deleted the added-appiumsetvalueoptions-to-hide-keyboard branch February 3, 2023 04:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants