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

Make it possible to have a different API for cls / className / classNames #32

Closed
raquo opened this issue Jun 10, 2018 · 4 comments
Closed

Comments

@raquo
Copy link
Owner

raquo commented Jun 10, 2018

As described in raquo/Laminar#22, consuming libraries might want to provide a special way to deal with CSS classes which is different from how they deal with other attributes.

It would benefit the consuming libraries if they could choose whether to keep the default cls / className / classNames API, but currently it's not possible.

I guess the most obvious solution to this problem would be to extract css / className / classNames (and perhaps also styleAttr?) into a separate OpinionatedAttrs trait that the consuming library may choose to not use.

@cornerman
Copy link
Contributor

In outwatch, we also handle class attributes differently, because we make them accumulate multiple assigned values. Currently, we just override the className member in RefectedAttrs (https://github.com/OutWatch/outwatch/blob/master/src/main/scala/outwatch/dom/DomTypes.scala#L110).

Style is maybe another "special kind" of attribute where a different handling might be needed. How do we decide, which of the values needs a special handling and which one does not? Why do you not like overriding the value?

@raquo
Copy link
Owner Author

raquo commented Aug 24, 2018

Problem is, I want to be able to do pass both Observable[String] and Observable[Map[String, Boolean]] to the <-- method, but without allowing a custom type for cls in SDT that's not possible because with a String codec Laminar's ReactiveClassAttr expects the <-- method to accept Observable[String], whereas I need it to accept Observable[String | Map[String, Boolean]] as I can't create another <-- method that would accept Observable[Map[String, Boolean]] because of type erasure. (I hope that because-chain is not too confusing, lol)

I guess I could work around this in Laminar with e.g. implicit value classes, but I'm not a fan of using implicits for such a popular use case. Besides, it's merely coincidental that I want to retain a string-based API in this case, and if I didn't want that I wouldn't have been able to accomplish what I want at all.

I'm working on this now on Laminar side, we'll see how it goes...

@cornerman
Copy link
Contributor

I see, that is indeed a good use case! In your dsl, you cannot diverge from the type signature of SDT and you are forced to expose it. Only changing semantics is possible.

Totally agree with your initial approach of using an OpinionatedAttr trait then. At least, we can then circumvent the problem for some of the likely candidates.

@raquo
Copy link
Owner Author

raquo commented Aug 25, 2018

Alright, coming soon as v0.9. Existing consuming libraries will just need to mix in two more traits (HTML and SVG) to retain current functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants