Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 85 additions & 1 deletion src/Behat/Mink/Element/NodeElement.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,91 @@ public function getParent()
{
return $this->find('xpath', '..');
}


/**
* Returns All child elements(including descendant) to the current one.
*
* @return NodeElements|null
*/
public function getAllChildren()
Copy link
Member

Choose a reason for hiding this comment

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

Please correct indentation. I guess you're using TABs here (in in other places in this PR), which doesn't comply with PSR-2.

{
return $this->findAll('xpath', 'descendant::*');
}

/**
* Returns All direct child elements(NOT including deeper descendant) to the current one.
*
* @return NodeElements|null
*/
public function getDirectChildren()
{
return $this->findAll('xpath', 'child::*');
}

/**
* Returns the first child element to the current one.
*
* @return NodeElement|null
Copy link
Member

Choose a reason for hiding this comment

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

I recommend using static instead of NodeElement here and in other methods, That would resolve to NodeElement as well by documentation generator and IDE.

*/
public function getFirstChild()
{
$elements = $this->findAll('xpath', 'child::*');
return $elements[0];
Copy link
Member

Choose a reason for hiding this comment

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

Just replacing findAll with find would do the trick. No need to get 0 element manually.

}

/**
* Returns the last direct child element to the current one.
*
* @return NodeElement|null
*/
public function getLastChild()
{
$elements = $this->findAll('xpath', 'child::*');
return $elements[count($elements) - 1];
Copy link
Member

Choose a reason for hiding this comment

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

The return end($elements); would do this in more obvious way.

}

/**
* Returns all preceding sibling elements to the current one.
*
* @return NodeElements|null
*/
public function getPreviousSiblings()
{
return $this->findAll('xpath', 'preceding-sibling::*');
}

/**
* Returns the preceding sibling element close to the current one.
*
* @return NodeElement|null
*/
public function getPreviousSibling()
{
$elements = $this->findAll('xpath', 'preceding-sibling::*');
return $elements[count($elements) - 1];
}

/**
* Returns all following sibling elements to the current one.
*
* @return NodeElements|null
*/
public function getNextSiblings()
{
return $this->findAll('xpath', 'following-sibling::*');
}

/**
* Returns the following sibling element close to the current one.
*
* @return NodeElement|null
*/
public function getNextSibling()
{
$elements = $this->findAll('xpath', 'following-sibling::*');
return $elements[0];
}

/**
* Returns current node tag name.
*
Expand Down
26 changes: 25 additions & 1 deletion src/Behat/Mink/Element/TraversableElement.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,31 @@ public function findById($id)

return $this->find('named', array('id', $id));
}


/**
* Finds element by its name.
*
* @param string name element name
* @return NodeElement|NodeElements|null
Copy link
Member

Choose a reason for hiding this comment

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

The find method doesn't return several elements, so NodeElements should be removed.

*/
public function findByName($name)
{
$name = $this->getSelectorsHandler()->xpathLiteral($name);
return $this->find('xpath', ".//*[@name=$name]");
Copy link
Member

Choose a reason for hiding this comment

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

It appears that method is displaced. The actual xpath should be defined in the NamedSelector and used like find('named', array('name' => $name).

Copy link
Author

Choose a reason for hiding this comment

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

called this method public function find($selector, $locator) in .../Mink/src/Behat/Mink/Element/Element.php
'xpath' is a selector, like 'css'.

Copy link
Member

Choose a reason for hiding this comment

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

Can you please rephrase, I don't get what you mean.

Copy link
Author

Choose a reason for hiding this comment

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

@aik099 please refer to

public function getParent()
{
    return $this->find('xpath', '..');
}

in .../Mink/src/Behat/Mink/Element/NodeElement.php
I mean I can use the find method like find('xpath'...

Copy link
Member

Choose a reason for hiding this comment

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

Now it's odd. I still don't get what you mean.

Have you understood my original comment about NamedSelector usage?

Copy link
Member

Choose a reason for hiding this comment

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

This is what I mean: https://github.com/Behat/Mink/blob/master/src/Behat/Mink/Element/TraversableElement.php#L33

The findById method is in TraversableElement so the findByName should be.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, got you now.
But, "name" is not in the NamedSelector.
we can call as $this->find('named', array('id', $id));
or $this->find('named', array('button', $locator));
or $this->find('named', array('link', $locator));
But I haven't found: $this->find('named', array('name', $name));

Copy link
Member

Choose a reason for hiding this comment

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

Of course it's not there. I'm telling that you should add it there and then use it.

}

/**
* Finds elements by its tag.
*
* @param string tag element tag
* @return NodeElements|null
Copy link
Member

Choose a reason for hiding this comment

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

That would be NodeElement[] and not NodeElements if you aware of PHPDoc syntax.

*/
public function findByTag($tag)
{
$tag = $this->getSelectorsHandler()->xpathLiteral($tag);
Copy link
Member

Choose a reason for hiding this comment

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

I don't get. Why you need to perform Xpath-safe escaping of the string if it is a CSS selector?

Also since this method doesn't do anything special and people can really give any CSS string instead tag, then it creates more confusion, than benefits it can provide.

return $this->findAll('css', $tag);
}

/**
* Checks whether element has a link with specified locator.
*
Expand Down