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 TextInput.setText #90

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add TextInput.setText #90

wants to merge 1 commit into from

Conversation

alkedr
Copy link
Contributor

@alkedr alkedr commented Sep 7, 2015

No description provided.

* @param newValue New value for this text input.
*/
public void setText(String newValue) {
getWrappedElement().sendKeys(getClearCharSequence() + newValue);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think instead the getClearCharSequence() + should be added to TextInput.sendKeys method. This way everybody who wants to change input value has no need to prepend it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would break backward compatibility and would be counterintuitive.
Also there may be situations when you don't want to clear anything, you just want to press a key.

@artkoshelev
Copy link
Contributor

Add some tests please

@alkedr
Copy link
Contributor Author

alkedr commented Sep 8, 2015

There are tests for Form.fill() that already test all code that I changed.
It seems that FileInput, Radio and Select are also tested through Form.fill().

@artkoshelev
Copy link
Contributor

If you move logic of deleting text to setText method, you should move corresponding tests as well.

@Test
public void setTextShouldNotAddClearKeysForTextInputWithEmptyText() {
when(webElement.getAttribute("value")).thenReturn("");
textInput.setText("qwerty");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is assertion for this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@ham1
Copy link
Contributor

ham1 commented Jun 13, 2016

👍 (just need to fix merge issue)

@@ -37,6 +37,15 @@ public void sendKeys(CharSequence... keys) {
}

/**
* Sets this text input's value.
Copy link
Contributor

@ham1 ham1 Jun 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps be explicit about using getClearCharSequence() and not using clear()?

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.

5 participants