Deprecate keyPress, keyDown, keyUp in favor of pressKey.#831
Deprecate keyPress, keyDown, keyUp in favor of pressKey.#831mparker17 wants to merge 1 commit intominkphp:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #831 +/- ##
============================================
- Coverage 98.47% 97.20% -1.27%
- Complexity 345 353 +8
============================================
Files 23 23
Lines 983 1002 +19
============================================
+ Hits 968 974 +6
- Misses 15 28 +13
Continue to review full report at Codecov.
|
| */ | ||
| public function keyPress($xpath, $char, $modifier = null) | ||
| { | ||
| @trigger_error(sprintf('The method %s is deprecated as of 1.11 and will be removed in 2.0', __METHOD__), E_USER_DEPRECATED); |
There was a problem hiding this comment.
Is triggering a deprecation notice from a method, that throws an exception a good idea?
Maybe we should trigger a deprecation notice in driver sub-classes, where this method is completely replaced?
The same goes for other similar places.
| * | ||
| * @param string|int|array $keys a string to type ('PHP Hypertext Preprocessor'), a integer key code (88), or an array of keys to press in sequence (e.g.: ["\u{E008}", 'H', "\u{E001}", 'ello']). See https://w3c.github.io/webdriver/#keyboard-actions | ||
| */ | ||
| public function pressKey($keys) { |
There was a problem hiding this comment.
Changing an interface is a BC break.
| public function keyPress($char, $modifier = null) | ||
| { | ||
| $this->getDriver()->keyPress($this->getXpath(), $char, $modifier); | ||
| $this->pressKey($this->wrapCharWithModifier($char, $modifier)); |
There was a problem hiding this comment.
Adding a @deprecated tag alone in the DocBlock won't help much, but having a matching deprecation notice triggering code will.
The same goes for other similar places in this class.
| public function keyPress($char, $modifier = null) | ||
| { | ||
| $this->getDriver()->keyPress($this->getXpath(), $char, $modifier); | ||
| $this->pressKey($this->wrapCharWithModifier($char, $modifier)); |
There was a problem hiding this comment.
keyPress, keyDown and keyUp are not something that can be implemented on top of pressKey, which is the priammy reason to deprecate them.
| /** | ||
| * Press one or more keyboard keys on an element. | ||
| * | ||
| * @param string|int|array $keys a string to type ('PHP Hypertext Preprocessor'), a integer key code (88), or an array of keys to press in sequence (e.g.: ["\u{E008}", 'H', "\u{E001}", 'ello']). See https://w3c.github.io/webdriver/#keyboard-actions |
There was a problem hiding this comment.
our own API should not be based on the Webdriver representation of modifier keys, as that might not fit other drivers.
3 participants
Problem / motivation
in #828, @stof said that "the [driver methods] that should actually be deprecated are the separate keydown, keypress and keyup methods in favor of a single pressKey method to match what is actually possible in browsers."
Proposed resolution
DriverInterface::keyDown(),DriverInterface::keyPress(), andDriverInterface::keyUp()in favor of a newDriverInterface::pressKey()NodeElement::keyDown(),NodeElement::keyPress(), andNodeElement::keyUp()in favor of a newNodeElement::pressKey()Notes
I modelled the
DriverInterface::pressKey()method after php-webdriver/php-webdriver's\Facebook\WebDriver\Remote\RemoteKeyboard::sendKeys()