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

fix: move caret even with clear backspace #2066

Merged
merged 7 commits into from
Jul 16, 2018
Merged

fix: move caret even with clear backspace #2066

merged 7 commits into from
Jul 16, 2018

Conversation

cyrilchampier
Copy link
Contributor

No description provided.

@twalpole
Copy link
Member

Hmmm -- It's a shame there's no cross browser/cross platform keystroke we can use for this instead of setSelectionRange

@session.execute_script("var el = document.getElementById('test_field'); el.focus(); el.setSelectionRange(0, 0);")
@session.first('//input').set('')
expect(@session.first('//input').value).to eq('')

@session.first('//input').set('monkey')
Copy link
Member

Choose a reason for hiding this comment

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

This test can't go here. The clear: :backspace behavior is Selenium specific so the test needs to go in spec/shared_selenium_session.rb with the other tests for that behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, I'll move the test tomorrow. Appart from that, are you ok with the PR?

Copy link
Member

@twalpole twalpole Jul 10, 2018

Choose a reason for hiding this comment

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

Looking at it more -- it appears the issue is Chrome specific -- wondering whether it's worth fixing it only in chrome_node.rb - I am curious why it only affects Chrome though since it seems like setting the caret to the beginning should affect FF too, and as I implied in my other comment I'd really prefer some way of doing this that doesn't depend on checking the element type and using setSelectionRange if possible.

@twalpole
Copy link
Member

twalpole commented Jul 10, 2018

Changing set_text to do native.send_keys(*([:end] + backspaces + [value.to_s])) fixes this (at least on MacOS) without having to resort to execute_script -- if it works on other platforms too it would be my preferred solution since it removes the field type limitation of setSelectionRange

@twalpole
Copy link
Member

twalpole commented Jul 10, 2018

If using :end doesn't work on other platforms it should be possible to check driver.browser.capabilities.platform or driver.browser.capabilities[:platform] or driver.browser.capabilities[:platform_name] and prepend the correct key for other platforms.

@cyrilchampier
Copy link
Contributor Author

:end seems a much better way to move caret indeed!
I only have a mac to test, if travis is green, can we consider it ok ?

@twalpole
Copy link
Member

twalpole commented Jul 11, 2018

@cyrilchampier I don't really understand why sending :end actually works, but if it works on mac and travis I'm fine considering it ok. Please remove the extraneous :focus_ and squash the commits down to 1 and I'll merge

@twalpole
Copy link
Member

@cyrilchampier Do you have time to make the requested cleanup here so I can get this merged? If not I'll go ahead and make the changes tomorrow.

@cyrilchampier
Copy link
Contributor Author

cyrilchampier commented Jul 15, 2018 via email

@twalpole twalpole merged commit 1a5f290 into teamcapybara:master Jul 16, 2018
@cyrilchampier
Copy link
Contributor Author

tx, I was getting married this week end, a bit busy ;)

@twalpole
Copy link
Member

Congratulations!

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