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

added pressKey, deprecated keyPress/keyDown/keyUp #772

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

oleg-andreyev
Copy link

Ref.: minkphp/MinkSelenium2Driver#304 (comment)

that's not the same API than previously. After calling keyDown, your implementation already released the key.

I think we should rather add a new pressKey method in Mink, and deprecate these 3 methods. And the new version of the driver would then implement only pressKey.

And we should implement the new method in 1.x too
The old implementation was not good anyway, as it was not actually pressing the key, but only triggering the JS actions (so the default behavior of pressing a key would not happen).

src/Element/NodeElement.php Show resolved Hide resolved
src/Element/NodeElement.php Show resolved Hide resolved
@stof
Copy link
Member

stof commented Jan 14, 2019

please also open a PR for the driver-testsuite repo to cover pressKey

@oleg-andreyev
Copy link
Author

@aik099
Copy link
Member

aik099 commented Sep 3, 2019

I recommend releasing all current features as 1.7.2 version and only then merging this PR which will force next release to be 1.8.0.

@oleg-andreyev
Copy link
Author

@aik099 I’m okay 1.7.2 will just PR probably tomorrow

@aik099
Copy link
Member

aik099 commented Sep 4, 2019

@aik099 I’m okay 1.7.2 will just PR probably tomorrow

@oleg-andreyev , according to #725 @greg-1-anderson is also wanting to help out with a PR. I'll let him do that.

@greg-1-anderson
Copy link

I took a swag at making an updated changelog in #786. If anyone wants to improve it, feel free; I am unfamiliar with the conventions used in this project, and just made my best effort based on previous entries.

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.

4 participants