Skip to content

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

Merged
merged 1 commit into from
Nov 9, 2015
Merged

Get Element Size. #528

merged 1 commit into from
Nov 9, 2015

Conversation

SergiuTudos
Copy link
Contributor

No description provided.

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:
Copy link
Contributor

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.

Copy link
Contributor Author

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
.

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 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...

Copy link
Contributor Author

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 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...


Reply to this email directly or view it on GitHub
https://github.com/robotframework/Selenium2Library/pull/528/files#r44123186
.

@SergiuTudos
Copy link
Contributor Author

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)
Copy link
Contributor

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.

@aaltat
Copy link
Contributor

aaltat commented Nov 6, 2015

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
@SergiuTudos
Copy link
Contributor Author

BINGO

aaltat added a commit that referenced this pull request Nov 9, 2015
@aaltat aaltat merged commit e061272 into robotframework:master Nov 9, 2015
@aaltat
Copy link
Contributor

aaltat commented Nov 9, 2015

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.
Copy link
Contributor

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.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed.

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.

5 participants