templ icon indicating copy to clipboard operation
templ copied to clipboard

docs: add more examples for conditional css classes

Open aranw opened this issue 1 year ago • 8 comments

I have some code that adds css classes depending on the current page and I noticed that the class wasn't being added

I've managed to create an example that reproduces the issue here

Tried to chance down the exact reason and I think it has something to do with the css processor where it adds them to the list of classes to be enabled here https://github.com/a-h/templ/blob/cdb04a64900625f9d819857ed8988481aaaec80c/runtime.go#L163

aranw avatar Oct 01 '23 16:10 aranw

The issue is that you're expecting the slice to be a logical OR of the truth values per class, when class assignment (currently) behaves more like the Object.assign operator in JavaScript.

Simplifying your example to the minimum, the last value for "a" here is false, so class a is not added to the output, whereas you expect that since one of the values is true, then the class should be output.

<div class={ templ.KV("a", true), templ.KV("a", false) }></div>

Rewriting your example (note the shouldBeGray function).

package main

templ Main(current_page string) {
	<!DOCTYPE html>
	<html
 		lang="en"
 		class={ 
		    "h-full",
		    templ.KV("bg-gray-50", shouldBeGray(current_page)),
		 }
	>
		<head>
			<meta charset="utf-8"/>
			<link rel="icon" href="/favicon.png"/>
			<meta name="viewport" content="width=device-width"/>
			<style type="text/css">
			.h-full {
				height: 100%;
			}
		</style>
		</head>
		<body class="h-full">
			Hello World!
		</body>
	</html>
}
package main

import (
	"net/http"

	"github.com/a-h/templ"
)

func shouldBeGray(page string) bool {
	return page == "login" || page == "register"
}

func main() {
	http.Handle("/", templ.Handler(Main("login")))

	http.ListenAndServe(":8080", nil)
}

Then you get the expected result.

<!doctype html>
<html lang="en" class="h-full bg-gray-50">
   <head>
      <meta charset="utf-8">
      <link rel="icon" href="/favicon.png">
      <meta name="viewport" content="width=device-width">
      <style type="text/css">
         .h-full {
         height: 100%;
         }
      </style>
   </head>
   <body class="h-full">Hello World!</body>
</html>

So, I guess it depends on how you view class assignment. I picked the concept of object assignment as the model to go with, but obviously, you assumed it would work differently.

I want to make templ intuitive for people, so doing what other things do is reasonable.

Looking at React, the React documentation at https://react.dev/reference/react-dom/components/common#how-to-apply-multiple-css-classes-conditionally points to the use of https://github.com/JedWatson/classnames for this type of assignment.

The behaviour of the classnames NPM package is that this program:

const classNames = require('classnames');
let output = classNames({ 'foo': true }, { 'foo': false });
console.log(output);

Prints foo, and this...

const classNames = require('classnames');
let output = classNames({ 'foo': true }, { 'foo': true });
console.log(output);

Prints foo foo.

I think that, based on the prior art in the React ecosystem, it would be reasonable to adjust the behaviour to use OR to set the class name, in which case templ would have output just one class name of foo, not two.

@joerdav - thoughts on this one?

a-h avatar Oct 01 '23 20:10 a-h

@a-h thanks for looking at this!

I totally didn't think that it worked that way and did not think to try it out a different way.

By the way I'm really liking using templ and I've been replacing my previous templating solution that was just using Go templates and a custom "engine" that I built that loads all the templates using go:embed

aranw avatar Oct 01 '23 21:10 aranw

I appreciate you giving it a proper go, and taking the time to raise good issues! Thanks a lot.

It's good to hear what people run into when they start using templ, so we can smooth off any rough edges. Everyone has different prior experience, so they have different expectations. Making templ non-surprising is a goal!

a-h avatar Oct 01 '23 21:10 a-h

This is semi related to this issue but I noticed another issue and perhaps this could all be solved another way?

I used the templ.KV perhaps not how was intended and I tried to apply multiple css classes and I didn't realise it was broken until now

<a
    href={templ.URL("/")}
    class={
        templ.KV("border-indigo-500 text-gray-900", isCurrentPage(settings.CurrentPage, "/")),
        templ.KV("border-transparent text-gray-500 hover:border-gray-300 hover:text-gray-700", !isCurrentPage(settings.CurrentPage, "/")),
        "inline-flex items-center border-b-2 px-1 pt-1 text-sm font-medium",
    }
    >Dashboard</a>

Maybe there could be a different function for this templ.Classes (naming things is hard) but maybe it could work in a way to solve this issue?

If the condition evaluates to true then it applies those classes to the element

aranw avatar Oct 02 '23 17:10 aranw

I'll pick up on your latest point tomorrow, but I noticed that the classnames library has a dedupe version which behaves just like my suggestion:

There is an alternate version of classNames available which correctly dedupes classes and ensures that falsy classes specified in later arguments are excluded from the result set.

From reading the PR notes, it looks the dedupe version in JS is 5x slower than the "standard" JS version, so they stuck with the existing version that doesn't de-dupe.

a-h avatar Oct 02 '23 17:10 a-h

My first impression on this one is that the following doesn't seem intuitive to me, as the KV function to me implies a unique key:

    <html lang="en" class={
            "h-full",
            templ.KV("bg-gray-50", current_page == "login"),
            templ.KV("bg-gray-50", current_page == "register"),
        }>

For readability sake I think this makes more sense:

	<html
 		lang="en"
 		class={ 
		    "h-full",
		    templ.KV("bg-gray-50", shouldBeGray(current_page)),
		 }
	>

There's also the possibility to consolidate if you don't want to separate out a function, this way it's up to the user to define if it's OR or AND:

    <html lang="en" class={
            "h-full",
            templ.KV("bg-gray-50", current_page == "login" || current_page == "login"),
        }>

joerdav avatar Oct 03 '23 07:10 joerdav

For me I think some improved docs or even a helpful error in the case of duplicate keys might be useful.

joerdav avatar Oct 03 '23 07:10 joerdav

Yes, I think https://templ.guide/syntax-and-usage/css-style-management#dynamic-class-names needs a lot of examples added to show the expected outputs based on a set of inputs.

I'll take that one on.

a-h avatar Oct 03 '23 08:10 a-h