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

Could you add additional methods: byCss/byCssSelector, byClassName for class Selectors? #360

Closed
rmarinsky opened this Issue Jul 24, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@rmarinsky
Contributor

rmarinsky commented Jul 24, 2016

It's will be usefull for dev which store locators like:
By buttonAddTocart= By.cssSelector("button.btn-addToCart");
By buttonAddTocart= By.className("btn-addToCart");

@asolntsev

This comment has been minimized.

Show comment
Hide comment
@asolntsev

asolntsev Jul 25, 2016

Member

Hi @bravoaqa!
I don't understand. Why can't you just use simpler form?

String buttonAddTocart= "button.btn-addToCart";
String buttonAddTocart= ".btn-addToCart";

Do you use page objects?

If yes, both options are bad.
What you actually need is a method in page object:

public class CartPage {
  public void addToCart(String itemName) {
    $("#items").find(byText(itemName).click();
    $("button.btn-addToCart").click();
  }
}

So, you don't need to have those By constants.

Member

asolntsev commented Jul 25, 2016

Hi @bravoaqa!
I don't understand. Why can't you just use simpler form?

String buttonAddTocart= "button.btn-addToCart";
String buttonAddTocart= ".btn-addToCart";

Do you use page objects?

If yes, both options are bad.
What you actually need is a method in page object:

public class CartPage {
  public void addToCart(String itemName) {
    $("#items").find(byText(itemName).click();
    $("button.btn-addToCart").click();
  }
}

So, you don't need to have those By constants.

@rmarinsky

This comment has been minimized.

Show comment
Hide comment
@rmarinsky

rmarinsky Jul 25, 2016

Contributor

Actually I could use object String for some locators, but more appropriate object for locators it Selectors or By. Dev can think that String ".btn-addToCart" it is variable

Some locators are reusing between different test methods.

Contributor

rmarinsky commented Jul 25, 2016

Actually I could use object String for some locators, but more appropriate object for locators it Selectors or By. Dev can think that String ".btn-addToCart" it is variable

Some locators are reusing between different test methods.

@yashaka

This comment has been minimized.

Show comment
Hide comment
@yashaka

yashaka Jul 25, 2016

Contributor

I believe that original arguments for this feature request do not make much sense.
Because

while

By buttonAddTocart= By.cssSelector("button.btn-addToCart");
By buttonAddTocart= By.className("btn-addToCart");

is imho better than

String buttonAddTocart= "button.btn-addToCart";
String buttonAddTocart= ".btn-addToCart";

because when you add xpath as a string, it will be hard to define in usage - which "string" locator has xpath and which - css.

But the following is better than both mentioned above:

SelenideElement buttonAddTocart= $("button.btn-addToCart");
SelenideElement buttonAddTocart= $(".btn-addToCart");

Still, I believe that just for consistency it's better to implement this feature...
It's just weird we have byId but do not have byCss, byClassName, etc...

Contributor

yashaka commented Jul 25, 2016

I believe that original arguments for this feature request do not make much sense.
Because

while

By buttonAddTocart= By.cssSelector("button.btn-addToCart");
By buttonAddTocart= By.className("btn-addToCart");

is imho better than

String buttonAddTocart= "button.btn-addToCart";
String buttonAddTocart= ".btn-addToCart";

because when you add xpath as a string, it will be hard to define in usage - which "string" locator has xpath and which - css.

But the following is better than both mentioned above:

SelenideElement buttonAddTocart= $("button.btn-addToCart");
SelenideElement buttonAddTocart= $(".btn-addToCart");

Still, I believe that just for consistency it's better to implement this feature...
It's just weird we have byId but do not have byCss, byClassName, etc...

@rmarinsky

This comment has been minimized.

Show comment
Hide comment
@rmarinsky

rmarinsky Jul 26, 2016

Contributor

I'm agree with you "It's just weird we have byId but do not have byCss, byClassName, etc..."

Contributor

rmarinsky commented Jul 26, 2016

I'm agree with you "It's just weird we have byId but do not have byCss, byClassName, etc..."

@asolntsev

This comment has been minimized.

Show comment
Hide comment
@asolntsev

asolntsev Jul 27, 2016

Member

Ok, let's do it.
How is ready to make a pull request? :)

On Jul 26, 2016 11:41 AM, "roma marinsky" notifications@github.com wrote:

I'm agree with you "It's just weird we have byId but do not have byCss,
byClassName, etc..."


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#360 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AARE3d7kdCmB-3Ad6diCvQUuGGeaaKlTks5qZcgdgaJpZM4JToWu
.

Member

asolntsev commented Jul 27, 2016

Ok, let's do it.
How is ready to make a pull request? :)

On Jul 26, 2016 11:41 AM, "roma marinsky" notifications@github.com wrote:

I'm agree with you "It's just weird we have byId but do not have byCss,
byClassName, etc..."


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#360 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AARE3d7kdCmB-3Ad6diCvQUuGGeaaKlTks5qZcgdgaJpZM4JToWu
.

@asolntsev asolntsev added this to the 3.8 milestone Aug 3, 2016

@asolntsev asolntsev self-assigned this Aug 3, 2016

@asolntsev asolntsev closed this in 04f5abe Aug 3, 2016

BorisOsipov added a commit to BorisOsipov/selenide that referenced this issue Nov 23, 2016

Closes #360 add methods Selectors.byCssSelector(), byClassName()
I am still not sure that this is really needed. But let it be...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment