-
Notifications
You must be signed in to change notification settings - Fork 775
Get Element Size. #528
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
Get Element Size. #528
Conversation
if element is None: | ||
raise AssertionError("Element with locator '%s' was not found or is not visible" % (locator)) | ||
|
||
if element.size['width'] * element.size['height'] == 0: |
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 am not sure, is it a good idea to raise an error if size is zero? Selenium seems to return zero size, if the element is hidden and I am not totally sure is it a good idea to change that idea. But raising an error follows Selenium2Library general idea. So we need discuss which way to go.
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.
this is was a request I've been working on. previous version did not raise
any exception and pull request has been rejected.
On Thu, Nov 5, 2015 at 8:43 PM, Tatu Aalto [email protected] wrote:
In src/Selenium2Library/keywords/_element.py
https://github.com/robotframework/Selenium2Library/pull/528#discussion_r44050715
:@@ -297,6 +297,21 @@ def get_horizontal_position(self, locator):
raise AssertionError("Could not determine position for '%s'" % (locator))
return element.location['x']
- def get_element_size(self, locator):
"""Returns width and height of element identified by `locator`.
The element width and height is returned.
Fails if a matching element is not found.
"""
element = self._element_find(locator, True, False)
if element is None:
raise AssertionError("Element with locator '%s' was not found or is not visible" % (locator))
if element.size['width'] \* element.size['height'] == 0:
I am not sure, is it a good idea to raise an error if size is zero?
Selenium seems to return zero size, if the element is hidden and I am not
totally sure is it a good idea to change that idea. But raising an error
follows Selenium2Library general idea. So we need discuss which way to go.—
Reply to this email directly or view it on GitHub
https://github.com/robotframework/Selenium2Library/pull/528/files#r44050715
.
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 don't believe I contributed to the earlier discussion about zero height/width but I am thinking the same as @aaltat that we don't want to error out on a zero width or height. Here, if the element is found, I would return the width and height and let the user interpret the result. I googled "can element have zero width or height" and it looks like there are legitimate reason for the result to be zero and even possible for either one to be zero and the other non zero thus failing on the
if width*height == 0 then error
check. Thus I would leave it up to the user to decide what is an error or not. Just my 2 cents...
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 think I'll agree. will change the test and the code itself
On Fri, Nov 6, 2015 at 12:32 PM, Ed Manlove [email protected]
wrote:
In src/Selenium2Library/keywords/_element.py
https://github.com/robotframework/Selenium2Library/pull/528#discussion_r44123186
:@@ -297,6 +297,21 @@ def get_horizontal_position(self, locator):
raise AssertionError("Could not determine position for '%s'" % (locator))
return element.location['x']
- def get_element_size(self, locator):
"""Returns width and height of element identified by `locator`.
The element width and height is returned.
Fails if a matching element is not found.
"""
element = self._element_find(locator, True, False)
if element is None:
raise AssertionError("Element with locator '%s' was not found or is not visible" % (locator))
if element.size['width'] \* element.size['height'] == 0:
I don't believe I contributed to the earlier discussion about zero
height/width but I am thinking the same as @aaltat
https://github.com/aaltat that we don't want to error out on a zero
width or height. Here, if the element is found, I would return the width
and height and let the user interpret the result. I googled "can element
have zero width or height" and it looks like there are legitimate reason
for the result to be zero and even possible for either one to be zero and
the other non zero thus failing on theif width*height == 0 then error
check. Thus I would leave it up to the user to decide what is an error or
not. Just my 2 cents...—
Reply to this email directly or view it on GitHub
https://github.com/robotframework/Selenium2Library/pull/528/files#r44123186
.
Code is ready. just accept it :) |
The element width and height is returned. | ||
Fails if a matching element is not found. | ||
""" | ||
element = self._element_find(locator, True, False) |
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.
It could be useful to put the required
argument as True
. In that way one does not need the lines 307 and 308. Also it quarentees unified error message.
We are getting quite close. Squashing all the separate commits to a single commit is desirable. Details can be example found from here: http://eli.thegreenplace.net/2014/02/19/squashing-github-pull-requests-into-a-single-commit |
…ll fail comment updated as pe suggestion from aaltat: Because it is not defined anywhere. I would just say: The element width and height is returned. An update for tests. In case element is doesn't have width or height method must fail. Example for a <div></> which does not contain anything Empty div will have height=0, and width will be equal to screen size. Empty div will have height=0, and width will be equal to screen size. Updates advised by Tatu Aalto Forgot to update test
BINGO |
I think this is good to go. |
"""Returns width and height of element identified by `locator`. | ||
|
||
The element width and height is returned. | ||
Fails if a matching element is not found. |
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.
Would be nice if there was an example like with Get Window Size.
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.
Agreed.
No description provided.