dalekjs / dalek

[unmaintained] DalekJS Base framework
MIT License
695 stars 63 forks source link

Feature request: jquery hasClass eqivalent #128

Open burt202 opened 10 years ago

burt202 commented 10 years ago

Hi, just started using DalekJS and all is good so far so thanks. I have a very small, possible very simple, feature request. I want to be able to do something like:

 test.assert.hasClass('#element', 'class', 'element has class')

and maybe also

 test.assert.notHasClass('#element', 'class', 'element doesnt have class')

I know i could do test.assert.attr('#element', 'class').to.contain('show') but that ist quite right and if that element had a class of showstopper and not show it would still pass.

Tell me if Ive missed something but hopefully this is something that you would consider adding in.

Cheers

krnlde commented 10 years ago

What about this, admittedly inconvenient, thing:

test
  .open('http://dalekjs.com/guineapig/')
  .assert.attr('#dataDiv', 'class', 'wellImUpperUpperClassHighSociety')
  .done();

Next time check and read the friendly manual first, man: http://dalekjs.com/docs/assertions.html#meth-attr

burt202 commented 10 years ago

FYI I have read the manual (and deemed attr not suitable) and looked at all the issues raised to make sure I wasnt created a duplicate.

I think what you suggested will not work if an element has more than one class and you want to check if only one exists or not.

I just think having a hasClass assertion would be clean, familiar, and very useful to many. Looking forward to hearing your thoughts

krnlde commented 10 years ago

Good point, although I understand the doc that way, that it should suit your case: "You can also use attr() for checking if a class is existent"

Did you test it yourself for multiple classes?

As said, an admittedly inconvenient method.

burt202 commented 10 years ago

No attr doesnt handle multiple classes, as it clearly treats the class name just a string so with the following HTML.....

<li class="home active">test</li>

.assert.attr('li.home', 'class', 'active')       <----fails

 assert.attr('li.home', 'class', 'home active')  <---- passes

....which is obviously not quite what Im after

burt202 commented 10 years ago

Updated previous comment

krnlde commented 10 years ago

Awesome, thanks dude :+1:

mdix commented 10 years ago

Hey guys,

it's possible and the manual states how:

and you can also match a substring (e. g. a single class if more classes are on that elem) with to.contain():

  test
    .open('http://dalekjs.com/guineapig/')
    .assert.attr('#dataDiv', 'class').to.contain('upperUpperClass')
    .done();

So in your case:

<li class="home active">test</li>

.assert.attr('li.home', 'class').to.contain('active')       <----fails

should do the trick.

Best Marc

burt202 commented 10 years ago

Hi, thanks for the response. I was aware you could use to.contain (see original post) but that is not ideal for the following reason:

  <li class="home active">test</li>

  .assert.attr('li.home', 'class').to.contain('act')       <----passes

This passes because act is a substring of the class string but it not actually a class of the element - this is the problem!!

I had already acknowledged using to.contain was the work-around but this whole thread is about introducing a slightly different assertion similar to jquerys hasClass to make this more readable and less susceptible to these sorts of issues.

mdix commented 10 years ago

Hi,

thanks for your time.

I also think that there must be a assertion to check for one class out of many, but currently you'll have to go with to.contain for quite some time I guess, even if it's not a solution I'd rely upon with great confidence. :)

Maybe we can discuss the syntax here?

What about assert.hasClass('#elem', 'className') and assert.not.hasClass('#elem', 'className') ?

Best Marc

burt202 commented 10 years ago

Yep, that syntax looks absolutely fine by me. Like I said, I have a work-around for now but i think this would be a good, useful and hopefully simple assertion to add.

asciidisco commented 9 years ago

We will tackle this in the "super shiny major rewrite" of Dalek, which currently lives in this repo: https://github.com/dalekjs/skaro

But to be honest, as long as the workaround does its job, I don't think that I will spend too much effort, getting this in the "current" codebase.

Munter commented 9 years ago

How about just using classList? Most browsers support it by now. http://caniuse.com/classlist

asciidisco commented 9 years ago

The IE 8/9 incompatibility is a bit of a bummer, I would rather try and implement this based on Webdriver methods.

burt202 commented 9 years ago

If its in hand Im happy to close, as I have a workaround which is fine for the time being. What is the timeline for the next release? Is 0.0.9 === skaro?

asciidisco commented 9 years ago

I´ll reopen it, cause this is something we need to adress properly. 0.0.9 is the last release based on the current codebase, I actually do not know if we will give Skaro the version number 0.10.0 although it´s very likely.

The current progress of Skaro can be tracked here: https://github.com/dalekjs/skaro We hope to release a beta version in October, but take that with a grain of salt, there is much work that needs to be done.

burt202 commented 9 years ago

Cool. Thanks for the update. Your work on this project is highly appreciated by the way!

gskachkov commented 9 years ago

I've added PR to implement this feature https://github.com/dalekjs/dalek/pull/138. Possible it will help