feature: Add CSS Style DSL and Class toggle utilities to uni.dom#390
feature: Add CSS Style DSL and Class toggle utilities to uni.dom#390
Conversation
Summary of ChangesHello @xerial, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a type-safe CSS style DSL and utilities for toggling CSS classes, which significantly improves the developer experience for styling components in uni.dom. The implementation is well-structured and accompanied by a comprehensive set of tests. The changes are a great addition to the framework. I've provided a few suggestions to improve consistency in the new CssStyles DSL and to align the design document with the final implementation.
| // Reactive toggle - returns Rx that emits class or empty | ||
| def when[A](rx: RxOps[A])(using ev: A =:= Boolean): DomNode = | ||
| rx.map(v => when(ev(v))) |
There was a problem hiding this comment.
The implementation of the reactive when in CssClass.scala is simpler and more direct than what is described in this plan. The implementation uses Rx[Boolean] directly, which is clearer. Also, the plan's method body returns an Rx but the signature says DomNode, which is a mismatch. The actual implementation correctly wraps the result in Embedded.
It would be good to update this design document to reflect the final implementation for future reference.
| // Reactive toggle - returns Rx that emits class or empty | |
| def when[A](rx: RxOps[A])(using ev: A =:= Boolean): DomNode = | |
| rx.map(v => when(ev(v))) | |
| // Reactive toggle - returns a DomNode that reactively updates the class | |
| def when(rx: Rx[Boolean]): DomNode = | |
| Embedded(rx.map(cond => when(cond))) |
| object cls extends DomAttributeOf("class"): | ||
| def toggle(className: String): ClassToggle = ClassToggle(className) |
There was a problem hiding this comment.
The implementation of cls in HtmlAttrs.scala also includes a := operator for setting the class directly, for consistency with the new style DSL. This design document should be updated to include it.
| object cls extends DomAttributeOf("class"): | |
| def toggle(className: String): ClassToggle = ClassToggle(className) | |
| object cls extends DomAttributeOf("class"): | |
| def :=[V: EmbeddableAttribute](v: V): DomNode = apply(v) | |
| def toggle(className: String): ClassToggle = ClassToggle(className) |
| * `display := "flex"` for type-safe styling. | ||
| */ | ||
| object css: | ||
| def :=(value: String): DomAttribute = DomAttribute("style", value, append = true) |
There was a problem hiding this comment.
For consistency with StyleProperty and to make style composition more robust, it's a good practice to ensure the raw CSS string ends with a semicolon. The current implementation passes the raw string as-is, while StyleProperty always adds a semicolon. This can lead to unexpected behavior if a user provides a style string without a trailing semicolon when combining it with other styles.
By ensuring a trailing semicolon, we make the behavior more predictable and less reliant on the browser's cssText parsing leniency.
def :=(value: String): DomAttribute = {
val trimmed = value.trim
val finalValue = if (trimmed.nonEmpty && !trimmed.endsWith(";")) s"${trimmed};" else trimmed
DomAttribute("style", finalValue, append = true)
}312ec9a to
2e3a205
Compare
This improves developer experience when working with inline styles and
conditional CSS classes, following React/Vue conventions:
- Add `style` object that extends DomElement for dual purpose:
- CSS style builder: `style(style.display := "flex", style.gap := "8px")`
- <style> tag element: `style("body { margin: 0; }")`
- Add `style := "..."` for raw style strings (Tailwind-friendly)
- Add `cls.toggle("className") when condition` for conditional classes
- Support both static boolean and reactive Rx[Boolean] conditions
- 65+ type-safe CSS properties
Usage:
```scala
import wvlet.uni.dom.all.*
val isActive = Rx.variable(false)
div(
style(style.display := "flex", style.gap := "8px"),
style := "margin: 10px;", // raw string
cls := "base",
cls.toggle("active") when isActive
)
```
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2e3a205 to
bd1a15f
Compare
- Fix reactive class toggle to use direct attribute binding instead of Embedded(rx.map(...)) to avoid inserting text node markers that break :empty selectors and DOM child order - Fix style() overload ambiguity by requiring at least one StyleValue argument, resolving conflict with inherited apply(DomNode*) from DomElement Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update reactive class toggle to reflect direct attribute binding fix - Update style apply signature to require at least one argument - Add Review Learnings section documenting codex review fixes - Document design evolution from user feedback Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Add CSS Style DSL and Class toggle utilities following React/Vue conventions:
styleobject extendsDomElementfor dual purpose:style(style.display := "flex", style.gap := "8px")<style>tag element:style("body { margin: 0; }")style := "display: flex;"(Tailwind-friendly)cls.toggle("active") when conditionUsage
Design
The
styleobject extendsDomElement("style", ...)to:HtmlTags.stylewithout type conflicts<style>tagstyle.display := "flex"syntax for propertiesTest plan
🤖 Generated with Claude Code