edvin / tornadofx

Lightweight JavaFX Framework for Kotlin
Apache License 2.0
3.67k stars 269 forks source link

CSS DSL #80

Closed ruckustboom closed 8 years ago

ruckustboom commented 8 years ago

Just an idea: should there be a DSL for adding CSS? Either to a node or to the View.

edvin commented 8 years ago

Oh, and super nice work, I really love the way this is shaping up!

ruckustboom commented 8 years ago

Sounds good

edvin commented 8 years ago

We should probably also have special support for padding/insets, like the box() function in Kara.

ruckustboom commented 8 years ago

I've made dimensions type safe (like in Kara), but it has one downside. All dimensions require a unit. You can no longer use fontSize = 14, you would have to do something like fontSize = 14.pt. Is that acceptable, or should I allow Any for linear dimensions?

(Current accepted units: px, mm, cm, inches, pt, pc, em, ex, percent)

edvin commented 8 years ago

Perfect! That is totally acceptable :)

ruckustboom commented 8 years ago

I've now got most of the properties added (with enums for the ones with limited choices). The only ones left to do are ones that I need to spend some more time figuring out how to do. Feel free to check out what I have so far. In the meantime, I'm going to bed :).

Also, anything with a fill can accept any Paint, not just Colors.

There are also linear dimensions (see above), box dimensions (basically just 4 linear dimensions (this is what the box() function uses (for padding, margin, border radius, etc))), angular dimensions (deg, rad, grad, turn), and temporal dimensions (s, ms).

edvin commented 8 years ago

Fantastic work! I really like the approach you have taken here. It seems to work really well. Please let me know if there are certain areas you need to discuss or need feedback on. I will start playing with this to see how it feels in the mean time.

edvin commented 8 years ago

By the way @t-boom - would you like commit access to the repo? You might as well integrate this stuff directly instead of going via a PR. We could start integrating the stuff and work together on it as well. I've put some stuff in CSS.kt so you would have to merge with that.

Let me know if it's OK with you, and I'll add you as a contributor on the project.

ruckustboom commented 8 years ago

Sure, that would probably make it easier for both of us.

I've merged in your changes (and pushed it up to my repo for now)

edvin commented 8 years ago

Hereby granted :) I'm thinking we could do another release as soon as this DSL is done - it's a major feature! Feel free to delegate work to me after you have checked in what you have so far :)

ruckustboom commented 8 years ago

Alright. I've pushed what I've got so far to the feature/css-dsl branch. I'll let you know what I need help with.

edvin commented 8 years ago

Cool! I just added a feature to SingleViewApp that lets you inline a stylesheet like this:

class CSSApp: SingleViewApp("CSS Test") {
    override val root = HBox()

    init {
        css {
            s(".box") { 
                backgroundColor = Color.BLUE
            }
        }

        with (root) {
            addClass("box")
        }
    }
}

I will try to integrate this into the feature/css-dsl branch to avoid a merge conflict later.

edvin commented 8 years ago

The change described above is now in the feature/css-dsl branch :)

I have a question about the use of extension functions. Take a look at this one for example:

fun <T> SelectionBlock.box(all: T) = CssBox(all, all, all, all)

Shouldn't this just be a member function of SelectionBlock? Same goes for some of the other extension functions - it seems like they could just as well be member functions. What do you think?

ruckustboom commented 8 years ago

Yes, they should. They were originally, but I pulled them out when I was running some tests and forgot to put them back in. I'll fix that.

edvin commented 8 years ago

OK! I also just made Stylesheet an open class like it was in your original design. I mistakenly changed it to abstract when I was playing around. It is nice to be able to instantiate a Stylesheet and then operate on it (see SingleViewApp for an example). Sorry about that :)

edvin commented 8 years ago

I saw you used com.sun.xml.internal.fastinfoset.util.StringArray in a couple of places. This class will be unavailable in JDK9. Did you use it to be able to check the type in toCSS?

If so, say the word and I'll try to implement a workaround :)

ruckustboom commented 8 years ago

That was another Accident :) That should have just been Array. I'll fix that too. Thanks!

edvin commented 8 years ago

Sweet! Let me know if you have stuff you want to delegate to me :)

ruckustboom commented 8 years ago

For properties that accept a URI, should I just accept in a String?

Also, there are a number of properties that accept a set of values plus some user defined thing. The cursor is a good example as it can accept from a predefined list or a uri to an image. What should I do with those?

edvin commented 8 years ago

String for URI sounds reasonable to me.

I'll think about the other issue for a bit. Wonder if Kara faced/solved similar issues...

ruckustboom commented 8 years ago

I haven't looked. I figured I'd get back to it later once I get the more tedious parts out of the way, but if you want to look into it, that would be awesome.

edvin commented 8 years ago

One possibility:

cursor: EnumValue
cursorUri: String

Haven't thought it through or tested how it feels, just throwing it out there :)

edvin commented 8 years ago

OK, I will have a look tonight and report back!

ruckustboom commented 8 years ago

That would work for the cursor, but there are others that are more complicated. -fx-background-position in Region is pretty bad.

edvin commented 8 years ago

The type should probably be javafx.scene.layout.BackgroundPosition and then we could add convenience functions later. What do you think?

ruckustboom commented 8 years ago

That's a good idea. Many of the others can probably be solved similarly.

edvin commented 8 years ago

Yes, it seems that way, including Cursor.

cursor = Cursor.CROSSHAIR
cursor = Cursor.cursor("http://coolcursors.com/cursor1.png")

To get the cursor name you would have to do something like:

        val cursorName = if (cursor is ImageCursor) {
            cursor.image.javaClass.getDeclaredField("url").let {
                it.isAccessible = true
                it.get(cursor.image)
            }
        } else {
            cursor.toString()
        }

A bit ugly, but it works :)

ruckustboom commented 8 years ago

We'll go with that.

Three more questions:

  1. Should I have all the enums follow standard naming conventions (ALL_CAPS)?
  2. Do you want to check to see which enums I've defined that can be replaced by existing JavaFX classes?
  3. Will is Enum work for enums defined with enum class ...?
edvin commented 8 years ago
  1. Yes, but mainly since the JavaFX enums are ALL_CAPS
  2. OK, I'll go over them
  3. Yes, it should, if not do java.lang.Enum::class.java.isAssignableFrom(x.javaClass)
ruckustboom commented 8 years ago

All properties have now been added, but not all render properly (I still need to finish font, effect and borderStyle). There are a few that still need validated better, and several that can probably be replaced with builtin classes / enums.

ruckustboom commented 8 years ago

As far as the skin property goes, the application will crash if you provide a skin name that doesn't exist. Should I require a skin class? I'm not sure how the skin property works.

edvin commented 8 years ago

Great! Looking for enums that can be avoided now. Yeah, we can require a skin class, I can't see how that should be a problem. Either it's on the classpath or it's not. In a pinch, if all the stylesheet author has is a String, a class could be generated with Class.forName("skinClass").kotlin.

edvin commented 8 years ago

Btw, just found a bug in SingleViewApp that i fixed in the master branch. Basically, both the JavaFX Application Launcher and the TornadoFX di framework created an instance of the App class. I'll apply the fix in this branch as well, just in case it bites us while working on this :)

ruckustboom commented 8 years ago

Is that why the CSS is always created twice?

edvin commented 8 years ago

Yes :) Can you verify that it is only created once if you pull my latest change?

ruckustboom commented 8 years ago

Just tried it. It seems to working great.

edvin commented 8 years ago

Phew :)

edvin commented 8 years ago

There are several enums that can probably be replaced. Two examples:

FXContentDisplay -> ContentDisplay FXTextOverrun -> OverrunStyle

It seems the enums can be used by converting the name to lowercase and switching underscore with dash.

If I understand this correctly, all we would have to do is check if the value is an enum in toCss and do tolowercase and replace _ with -.

Should I go ahead and try that for a couple of the enums?

ruckustboom commented 8 years ago

That was my thought too (thus my question above about checking is Enum). Go for it.

ruckustboom commented 8 years ago

Another (ugly) progress demo:

class CssTest : SingleViewApp("CSS Test") {
    override val root = VBox()

    init {
        css {
            val pad = mixin {
                padding = box(1.em)
                borderColor = box(LinearGradient(0.0, 0.0, 10.0, 10.0, false, CycleMethod.REFLECT, Stop(0.0, Color.RED), Stop(1.0, c(0.0, 1.0, 0.0))))
                borderWidth = box(5.px)
            }

            s(".box") {
                +pad
                backgroundColor = RadialGradient(90.0, 0.5, 0.5, 0.5, 0.25, true, CycleMethod.REFLECT, Stop(0.0, Color.WHITE), Stop(1.0, Color.BLACK))
                spacing = 5.px

                s(".label") {
                    +pad
                    fontSize = 14.pt
                    textFill = c("white")
                    backgroundColor = c("blue", 0.75)
                    rotate = .95.turn
                    translateX = .5.inches
                    minHeight = 6.em
                    scaleX = 2
                    scaleY = .75
                    backgroundRadius = box(25.px)
                    borderRadius = box(25.px)
                }
            }
        }

        with(root) {
            prefWidth = 400.0
            prefHeight = 400.0

            hbox {
                styleClass += "box"

                label("Alice")
                label("Bob")
            }
        }
    }
}

css-tests-ugly

.box {
    -fx-padding: 1em 1em 1em 1em;
    -fx-border-color: linear-gradient(from 0.0px 0.0px to 10.0px 10.0px, reflect, rgba(255, 0, 0, 1) 0.0%, rgba(0, 255, 0, 1) 100.0%) linear-gradient(from 0.0px 0.0px to 10.0px 10.0px, reflect, rgba(255, 0, 0, 1) 0.0%, rgba(0, 255, 0, 1) 100.0%) linear-gradient(from 0.0px 0.0px to 10.0px 10.0px, reflect, rgba(255, 0, 0, 1) 0.0%, rgba(0, 255, 0, 1) 100.0%) linear-gradient(from 0.0px 0.0px to 10.0px 10.0px, reflect, rgba(255, 0, 0, 1) 0.0%, rgba(0, 255, 0, 1) 100.0%);
    -fx-border-width: 5px 5px 5px 5px;
    -fx-background-color: radial-gradient(focus-angle 90.0deg, focus-distance 50.0% , center 50.0% 50.0%, radius 25.0%, reflect, rgba(255, 255, 255, 1) 0.0%, rgba(0, 0, 0, 1) 100.0%);
    -fx-spacing: 5px;
}
.box .label {
    -fx-padding: 1em 1em 1em 1em;
    -fx-border-color: linear-gradient(from 0.0px 0.0px to 10.0px 10.0px, reflect, rgba(255, 0, 0, 1) 0.0%, rgba(0, 255, 0, 1) 100.0%) linear-gradient(from 0.0px 0.0px to 10.0px 10.0px, reflect, rgba(255, 0, 0, 1) 0.0%, rgba(0, 255, 0, 1) 100.0%) linear-gradient(from 0.0px 0.0px to 10.0px 10.0px, reflect, rgba(255, 0, 0, 1) 0.0%, rgba(0, 255, 0, 1) 100.0%) linear-gradient(from 0.0px 0.0px to 10.0px 10.0px, reflect, rgba(255, 0, 0, 1) 0.0%, rgba(0, 255, 0, 1) 100.0%);
    -fx-border-width: 5px 5px 5px 5px;
    -fx-font-size: 14pt;
    -fx-text-fill: rgba(255, 255, 255, 1);
    -fx-background-color: rgba(0, 0, 255, 0.74902);
    -fx-rotate: 0.95turn;
    -fx-translate-x: 0.5in;
    -fx-min-height: 6em;
    -fx-scale-x: 2;
    -fx-scale-y: 0.75;
    -fx-background-radius: 25px 25px 25px 25px;
    -fx-border-radius: 25px 25px 25px 25px;
}
edvin commented 8 years ago

Fantastic :)

I'm running into some problems with checking if value: T is an enum in toCss. Either I have to make the function inline so that T can be reified, or I have to change the bounds of T to T : Any.

The first makes it impossible to recurse into toCss (needed for Array/Pair etc), and the other makes it necessary to perform some casts to Any when calling toCss from a class with type parameters, like CssBox.

Don't love any of those approaches. Will try some more.

ruckustboom commented 8 years ago

I'm not sure if I understand. Why won't

is Enum<*> -> return value.toString().toLowerCase().replace("_", "-")

work?

edvin commented 8 years ago

Haha, that works, I think I'm too tired, midnight here already :) I'll revert that crazyness and commit the removed enums etc.

edvin commented 8 years ago

What about this one?

null -> return ""  // This should only happen in a container TODO: Is there a better way to handle this?

Do you need this check at all? All passed in types are non-null.

ruckustboom commented 8 years ago

Yes, because anything inside a box, pair, array, etc. that is null somehow still makes it into that function without causing an error.

borderColor = box(Color.Green, null)

would create

-fx-border-color: rgba(0, 255, 0, 1) null rgba(0, 255, 0, 1) null;
edvin commented 8 years ago

Ah :) Just committed, I'm going to bed before I do any more harm :)

ruckustboom commented 8 years ago

Anything that accepts color has to accept Paint?. It was the only way to handle trying to create invalid colors (JavaFX throws an error, so I return a null and have the property setter ignore it; unfortunately the property setter can't look inside containers, so I had to put something there, and a blank string seems to do the least damage in CSS).

edvin commented 8 years ago

Sorry for being dense here, but I don't quite understand (even after a little sleep, hehe :)

var backgroundColor: Paint?

Why does it help to declare this as nullable? I don't understand the part about creating invalid colors :) Can you give me an example?

ruckustboom commented 8 years ago

When you use JavaFX's methods to create a color, it throws an exception if you give bad values, so I catch the exceptions and return null. If you can think of a good way to avoid nulls, that would be awesome, as we could then make cssprop type <reified T : Any>

edvin commented 8 years ago

Ah, OK, I will investigate and get back to you :)