Skip to content
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

Classes processing #32

Closed
ul opened this issue Dec 29, 2014 · 12 comments
Closed

Classes processing #32

ul opened this issue Dec 29, 2014 · 12 comments

Comments

@ul
Copy link
Contributor

ul commented Dec 29, 2014

Noticed two non-conveniences:

  1. {:class nil} produces class="null", but no classname is desired
  2. [:div.class1 {:class "class2"} produces class="class2", but merge is desired
@aaronc
Copy link
Owner

aaronc commented Dec 29, 2014

  1. Looks like a bug that we should fix
  2. This is possible but would add some code complexity I'm not sure is necessary. Can you give some more use cases?

@ul
Copy link
Contributor Author

ul commented Dec 30, 2014

  1. is only point of shortcut, when some classes are fixed, but another is dynamic, like [:div.win__content {:class (when mod? " win__content_mod")} — second class should be added only if mod? is true, first one should always present. Handy to have this sugar, but not really necessary.

@aaronc
Copy link
Owner

aaronc commented Jan 2, 2015

This may be fixed in cafc91c. Can you check?

@ul
Copy link
Contributor Author

ul commented Jan 3, 2015

I can't check this and diff because develop branch renders only part of dom tree in my app without any errors shown. Investigating this right now. Also non-reactively is noop now? invalidate-rx is undefined and not used. How to wrap reactive element without registering it as dependency now?

@ul
Copy link
Contributor Author

ul commented Jan 3, 2015

I guess ivalidate-rx now is called register-dep? If yes then we need to update macros.

@ul
Copy link
Contributor Author

ul commented Jan 3, 2015

Specifically, in [:div [:div ...]] only outermost tag is rendered.

@ul
Copy link
Contributor Author

ul commented Jan 3, 2015

Excuse me for my short reports, I have no enough time to dive into subject these days and I don't tracked all changes which you made in code in develop branch, so I don't clearly understand how to debug it.

@aaronc
Copy link
Owner

aaronc commented Jan 3, 2015

Oops! Well this was a pretty messy refactoring on my part - sorry. Hopefully in the latest commit I pushed on develop, this should be fixed. I'm hoping this will be the last major refactoring of the core API - I think this should make 3rd party integration much easier now though.

@ul
Copy link
Contributor Author

ul commented Jan 5, 2015

The result in my app is the same as before. Please see the code https://gist.github.com/ul/992bd5fcd1d27f06337f may be that will help.

@ul
Copy link
Contributor Author

ul commented Jun 10, 2015

in the latest development branch:

  1. bug still persist
  2. merge works fine

@aaronc
Copy link
Owner

aaronc commented Jun 10, 2015

I just pushed a fix which sets .className to "" when :class is nil. Does that seem correct?

@ul
Copy link
Contributor Author

ul commented Jun 10, 2015

Works fine, thank you!

@ul ul closed this as completed Jun 10, 2015
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

No branches or pull requests

2 participants