search

plumatic / dommy

A tiny ClojureScript DOM manipulation and event library
758 stars 74 forks source link

You can now add a class to an element that didn't already have a class. #63

Open JulianBirch opened 10 years ago

JulianBirch commented 10 years ago

Hi, it's probably possible to make this code more efficent/elegant, but it's passing my manual tests.

Sorry, no test case because I'm having trouble getting your tests to run.

cpetzold commented 10 years ago

Hey Julian, can you explain the problem this is solving with a code sample?

JulianBirch commented 10 years ago

(Not at a dev computer right now, so apologies if this is garbage)

<div id='hello'>World</div>
(dommy/add-class! (sel1 :#hello) :world)

should trigger it. The basic problem is that if the element doesn't have a class, then class-name returns null, which means you can't call .split on the result.

As I say, sadly your tests don't work on my machine... :(

jeluard commented 10 years ago

It might be specific to some browser? It's working fine for me without the patch.

JulianBirch commented 10 years ago

It is, the code I've fixed is on the "doesn't support classlist" branch. The joke being that it affected Chrome when I wrote it, but Chrome now seems to support classlist. So, it only really affects IE versions now.

Anyway, as I said, the problem is that if you access className and there isn't a class, it returns nil rather than empty string.

cpetzold commented 10 years ago

The split is on classes, not class-name. It should be fine if class-name returns nil. Can you paste an error?

Also, the tests should work now after switching over to clojurescript.test.

JulianBirch commented 10 years ago

Sorry, it's class-index that assumes that class-name is not null. It calls .index on it.