-
Notifications
You must be signed in to change notification settings - Fork 286
Implement the conversion of XPath to NodeElement in CoreDriver #633
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
Conversation
|
OK. |
|
this is how it will impact drivers regarding to the Mink versions:
|
src/Behat/Mink/Driver/CoreDriver.php
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% satisfied about the name of this method (it does not return Element objects after all). But I don't have a better idea. What about you @aik099 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it finds xpaths of elements within element with given xpath. We don't have much words to operate with here. Only find, element, xpath, but since xpath is implementation detail we can't use it method name.
Maybe we can call it findElementSelectors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also see an issue with this name: we also use the concept of Selector in Mink, and it is not exactly the same think than here (even though it is related)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matchElements() ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, my issue with the findElements name is the elements part, because it does not return elements objects. So matchElements is not better in this regard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method accepts xpaths and returns xpaths and it's unlikely, that this behavior will change any time soon. Maybe we should expose xpath word in method name.
Another set of possible names:
findElementXpathsfindElementLocatorsfindSubXpathsfindLocatorsFromXpath
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will go for findElementXpaths
f07ed9e to
1ccc391
Compare
|
@aik099 are we good now ? |
|
Yes. |
The goal is to ease the development of Mink 2.0 by allowing drivers to be compatible with both Mink 1.7 and 2.0 on a single code base. The only change required for drivers is now handled at the level of the CoreDriver shipped in Mink when drivers should to override findElementXpaths rather than find.
1ccc391 to
97123e0
Compare
Implement the conversion of XPath to NodeElement in CoreDriver
|
All driver PRs are now done |
The goal is to ease the development of Mink 2.0 by allowing drivers to be compatible with both Mink 1.7 and 2.0 on a single code base. The only change required for drivers is now handled at the level of the CoreDriver shipped in Mink when drivers should to override findElements rather than find (see https://github.com/minkphp/MinkZombieDriver/pull/58/files for what is required currently).
This will allow decoupling the 2.0 version of Mink from a new major version of drivers (the new major version of ZombieDriver should rather be related to the full rewrite suggested in minkphp/MinkZombieDriver#104 if it ever happens than to the Mink refactoring for instance)