search

jwstegemann / fritz2

Easily build reactive web-apps in Kotlin based on flows and coroutines.
https://www.fritz2.dev
MIT License
627 stars 25 forks source link

Grids don't render correctly #831

Closed JonathanMandel closed 6 months ago

JonathanMandel commented 6 months ago

Describe the bug I've written a web app that uses Tailwind CSS and Fritz2 1.0 RC11. It uses Tailwind grid commands to generate a spreadsheet-style grid. However, I've found that properly coded grids do not render correctly sometimes.

Here is the code that does part of the work. It renders the column headings as a 1-row grid.

`` fun RenderContext.columnHeadingFramework( parms: Store ) { // 4-column version div ("grid grid-cols-4 w-full gap-2 mt-0") { div("col-span-3") {} div("col-span-1") {} }

// 6-column version
div ("grid grid-cols-6 w-full gap-2 mt-0") {
    div("col-span-3") {}
    div("col-span-2") {}
    div("col-span-1") {}
}

// 10-column version
div ("grid grid-cols-10 w-full gap-2 mt-0") {
    div("col-span-4") {}
    div("col-span-2") {}
    div("col-span-2") {}
    div("col-span-1") {}
    div("col-span-1") {}
}

parms.data.render { p ->
    val gridCols = p.gridSpans.sum()
    div("w-full isolate grid grid-cols-$gridCols w-full gap-2 mt-6") {
        //div("") {
            for (i in p.labels.indices) {
                val label: String
                val labelFormat: String
                if (p.labels[i].first() == '*') {
                    labelFormat = "justify-self-center uppercase text-gray-700 text-xs font-bold"
                    label = p.labels[i].drop(1)
                } else {
                    labelFormat = "px-4 uppercase text-gray-700 text-xs font-bold"
                    label = p.labels[i]
                }
                console.log("columnHeadingFramework label: $label, col-span = ${p.gridSpans[i]}")
                div("col-span-${p.gridSpans[i]}") {
                    val s = p.tips.getOrNull(i)
                    if (s != null) {
                        label(labelFormat) {
                            `for`("grid-col-$i")
                            +label
                        }.tooltip(UI.tooltipStyle){
                            +s
                            placement = PlacementValues.top
                            arrow()
                        }
                    } else {
                        label(labelFormat) {
                            `for`("grid-col-$i")
                            +label
                        }
                    }
                }
            }
        //}
    }
}

} ``

Broken final table Not broken final table

These two images show what it's supposed to look like when it breaks and how it looks when it works.

To get the code to fail, comment out the empty div()s at the top of the function.

Now here's the odd thing. It doesn't always fail as soon as you build without those empty div()s. It may work for a few builds, then it fails. Putting the empty div()s back in reliably fixes the problem. The empty div()s match the grid patterns that get passed into this function.

I have tried every variant of this function I can imagine, including changing the composition of Store objects I pass into the function, and passing plain data (no Store). I've also tried a bare-bones version of the function, which exhibits the same behavior.

Desktop (please complete the following information):

Additional context You can see a recent working version of this app at www.bakersformulary.com.

haukesomm commented 6 months ago

Hi @JonathanMandel, thanks for reaching out with your issue!

In your code you are using String interpolation to dynamically construct Tailwind classes like this: grid-cols-$gridCols.

Tailwind class names must be present at compile time in order to be recognized and not be optimized out. By using String interpolation the final class name is only present at runtime which may result in the class not being part of your stylesheet depending on your configuration.

Does the problem persist if you replace your dynamically generated class names with static class names (e.g. grid-cols-4)? If this is the case, your issue may be caused by the mechanics described above.

JonathanMandel commented 6 months ago

Thank you for replying.

What you say makes sense. And there are two things that I don't understand.

First, in your own example code, you use string composition. Here's an example from TicTacToe:

div("row") { gameStore.field.renderEach { cell -> div("col-4 border d-flex justify-content-center align-items-center") { inlineStyle("height: 10rem; background-color: ${if (cell.isInWinningGroup) "green" else "white"};")

Please explain why this code works if "background-color: green" and "background-color: white" haven't been hardcoded?

Second, if there is no way to alter Tailwind formatting under program control, Tailwind is totally useless in production coding environments. Yet Tailwind is one of the most popular CSS libraries in existence. How are formats composed programmatically in the real world?

Thanks.

On Fri, Dec 29, 2023 at 6:29 AM Hauke Sommerfeld @.***> wrote:

Hi @JonathanMandel https://github.com/JonathanMandel, thanks for reaching out with your issue!

In your code you are using String interpolation to dynamically construct Tailwind classes like this: col-span-${p.gridSpans[i]}.

Tailwind class names must be present at compile time in order to be recognized and not be optimized out. By using String interpolation the final class name is only present at runtime which may result in the class not being part of your stylesheet depending on your configuration.

Does the problem persist if you replace your dynamically generated class names with static class names (e.g. grid-cols-4)? If so, your issue is probably caused by the mechanics described above.

— Reply to this email directly, view it on GitHub https://github.com/jwstegemann/fritz2/issues/831#issuecomment-1872131851, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYSCZWNYFLPMAWB5KHGTHLYL3HUJAVCNFSM6AAAAABBB7O6F2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZSGEZTCOBVGE . You are receiving this because you were mentioned.Message ID: @.***>

Lysander commented 6 months ago

You quoted an example that works with Inline styles, so no tailwindcss there, just plain old CSS!

We should rework that imho, as this could be solved in a better way without inline styles and obviously disturbs users.

You simply must use Kotlins control flow mechanisms like if...else or when to apply the classes that are needed on certain states.

Iirc there are examples in the documentation in the rendering chapter that explicitly deals with reactive styling.

JonathanMandel commented 6 months ago

Got it. So, can I use string composition, as long as the classes that might get composed are hardcoded elsewhere? For example, if I use "colspan-$cv", and I also (somewhere) have "colspan-3" and whatever other classes I generate? In other words, if I defeat the optimization by naming the classes I'm going to generate programmatically?

If I am required to use Kotlin control flow to generate Tailwind variants, my code is going to balloon considerably. "colspan-$cv" becomes

val fmt = when(cv) { 1 -> "colspan-1" 2 -> "colspan-2" 3 -> "colspan-3" 4 -> "colspan-4" 5 -> "colspan-5" 6 -> "colspan-6" 7 -> "colspan-7" 8 -> "colspan-8" 9 -> "colspan-9" 10 -> "colspan-10" else -> "colspan-0" }

or an array-based variant. Yes, I can hide those all in a support file, but is this actually the only way to do this?

On Fri, Dec 29, 2023 at 11:46 AM Christian Hausknecht < @.***> wrote:

You quoted an example that works with Inline styles, so no tailwindcss there, just plain old CSS!

We should rework that imho, as this could be solved in a better way without inline styles and obviously disturbs users.

You simply must use Kotlins control flow mechanisms like if...else or when to apply the classes that are needed on certain states.

Iirc there are examples in the documentation in the rendering chapter that explicitly deals with reactive styling.

— Reply to this email directly, view it on GitHub https://github.com/jwstegemann/fritz2/issues/831#issuecomment-1872304713, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYSCZUUOXUACWLF7GPKD5TYL4M2NAVCNFSM6AAAAABBB7O6F2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZSGMYDINZRGM . You are receiving this because you were mentioned.Message ID: @.***>

haukesomm commented 6 months ago

Exactly, currently this is the only way. I'm not sure if a more dynamic approach is easily doable at the moment.

Lysander commented 6 months ago

For example, if I use "colspan-$cv", and I also (somewhere) have "colspan-3" and whatever other classes I generate? In other words, if I defeat the optimization by naming the classes I'm going to generate programmatically?

You might use this pattern, but be cautious: The code that contains the tailwind class names must really be "useful" - if not, it will be deleted by the DCE. I would encourage you to not use String-interpolation to create CSS classes. It resembles premature optimization; read below.

If I am required to use Kotlin control flow to generate Tailwind variants, my code is going to balloon considerably. "colspan-$cv" becomes val fmt = when(cv) { 1 -> "colspan-1" 2 -> "colspan-2" 3 -> "colspan-3" 4 -> "colspan-4" 5 -> "colspan-5" 6 -> "colspan-6" 7 -> "colspan-7" 8 -> "colspan-8" 9 -> "colspan-9" 10 -> "colspan-10" else -> "colspan-0" } or an array-based variant. Yes, I can hide those all in a support file, but is this actually the only way to do this?

Do not have fear to integrate such class assigning code! We have done this quite often yet within some internal UI-component library. We believe this is no harm at all, but helps to keeps the UI code rather clean of String-"magic". Just put such code into a function; we have used this pattern as extension function of some enum class types quite often to assign each value its own color-class.

To recap this issue: It is not an issue with fritz2 imho, so you might close it, if there is nothing we could do.

JonathanMandel commented 6 months ago

You are right, this is not a problem with Fritz2. Thank you for taking the time to explain this. I will close the issue.

On Sun, Dec 31, 2023, 12:43 PM Christian Hausknecht < @.***> wrote:

For example, if I use "colspan-$cv", and I also (somewhere) have "colspan-3" and whatever other classes I generate? In other words, if I defeat the optimization by naming the classes I'm going to generate programmatically?

You might use this pattern, but be cautious: The code that contains the tailwind class names must really be "useful" - if not, it will be deleted by the DCE https://kotlinlang.org/docs/javascript-dce.html. I would encourage you to not use String-interpolation to create CSS classes. It resembles premature optimization; read below.

If I am required to use Kotlin control flow to generate Tailwind variants, my code is going to balloon considerably. "colspan-$cv" becomes val fmt = when(cv) { 1 -> "colspan-1" 2 -> "colspan-2" 3 -> "colspan-3" 4 -> "colspan-4" 5 -> "colspan-5" 6 -> "colspan-6" 7 -> "colspan-7" 8 -> "colspan-8" 9 -> "colspan-9" 10 -> "colspan-10" else -> "colspan-0" } or an array-based variant. Yes, I can hide those all in a support file, but is this actually the only way to do this?

Do not have fear to integrate such class assigning code! We have done this quite often yet within some internal UI-component library. We believe this is no harm at all, but helps to keeps the UI code rather clean of String-"magic". Just put such code into a function; we have used this pattern as extension function of some enum class types quite often to assign each value its own color-class.

To recap this issue: It is not an issue with fritz2 imho, so you might close it, if there is nothing we could do.

— Reply to this email directly, view it on GitHub https://github.com/jwstegemann/fritz2/issues/831#issuecomment-1873034788, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYSCZTE6ICAYCFLEXBGZULYMHE6TAVCNFSM6AAAAABBB7O6F2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZTGAZTINZYHA . You are receiving this because you were mentioned.Message ID: @.***>