Proposal for changes #9

javierav · 2024-10-05T05:51:37Z

After using this gem for some time to create components in the project I am working on, I have detected a series of improvements that could be interesting to apply to this library, so I am sharing them with you to decide if they are interesting to implement.

1. Configuration structure

The current structure is:

ClassVariants.build(
  "<base>",
  variants: {},
  defaults: {}
)

In order to be able to undertake the following proposals, the chain containing the base classes should be part of the configuration hash:

ClassVariants.build(
  base: {},
  variants: {},
  defaults: {}
)

2. Slot support

This is about implementing the slot concept as defined in Tailwind Variants. This great idea allows you to apply different styles to the different html tags that a more complex component can be divided into. It is especially useful when used with variants / compound variants as seen in the example of an alert message.

The implementation is simple, since you have to detect if the object passed to class is a text string or a hash.

If it is a string as until now, it will go to the default slot:

{
  base: "inline-flex items-center rounded",
  variants: {
    color: {
      red: "bg-red-600",
      blue: "bg-blue-600",
    }
  }
}

If instead of a string it is a hash, the keys of that hash will define the slots:

{
  base: {
    container: "rounded py-3 px-5 mb-4",
    title: "font-bold mb-1"
  },
  variants: {
    outlined: {
      container: "border"
    }
  },
  compoundVariants: [
    {
      type: :outlined, severity: :error,
      class: {
        container: "border-red-700 dark:border-red-500",
        title: "text-red-700 dark:text-red-500",
        message: "text-red-600 dark:text-red-500"
      }
    },
    {
      type: :outlined, severity: :success,
      class: {
        container: "border-green-700 dark:border-green-500",
        title: "text-green-700 dark:text-green-500",
        message: "text-green-600 dark:text-green-500"
      }
    },
    {
      type: :filled, severity: :error,
      class: {
        container: "border-red-700 dark:border-red-500",
        title: "text-red-700 dark:text-red-500",
        message: "text-red-600 dark:text-red-500"
      }
    },
    {
      type: :filled, severity: :success,
      class: {
        container: "bg-green-100 dark:bg-green-800",
        title: "text-green-900 dark:text-green-50",
        message: "text-green-700 dark:text-green-200"
      }
    }
  ]
}

type: :outlined, severity: :error will return the classes:

container slot: rounded py-3 px-5 mb-4 border border-red-700 dark:border-red-500
title slot: font-bold mb-1 text-red-700 dark:text-red-500
container slot: text-red-600 dark:text-red-500

When executing the render method, you can optionally pass it the slot from which you want to get the classes:

alert_classes.render(:container, color: :blue, size: :sm)

Although we will have to discuss implementation details later, because I have the feeling that something better needs to be done.

3. Override classes

When rendering classes, we need to be able to override them. Following the example above:

alert_classes.render(
  :container, color: :blue, size: :sm, class: "shadow"
)

4. Class mixing strategies

At various points in this library we find ourselves in the need to mix CSS classes from different sources, so different mixing strategies should be implemented.

The most basic one can be a simple concatenation as it happens now.
For Tailwind, GitHub - gjtorikian/tailwind_merge: Utility function to efficiently merge Tailwind CSS classes without style conflicts could be used, always as an optional feature.

5. Helper to include in other classes

It is convenient to provide a helper that can be included in other classes to add the use of this library in a simple way:

class MyClass
  include ClassVariants::Helper

  class_variants {
    base: {},
    variants: {}
  }
end

MyClass.new.class_variants(:container, color: :red, class: "shadow")

What do you think @adrianthedev?

The text was updated successfully, but these errors were encountered:

adrianthedev · 2024-10-05T06:18:49Z

Hey @javierav

Awesome suggestions! Thanks for taking the time to fill this in.

Yes! Let's do the new configuration structure (base:)
That's a great idea. I'd love to support that, but my gut tells me we need a better format for declaring those classes. I mean, a hash can do so much until it becomes unreadable and difficult to understand. I'll post mode below.
Yes! Let's add class too.
Yes. I saw that gem and I agree, we should have that added to class_variants.
Yes! I'd love something like that.

For 2. I think we should explore a new API.
Hashes are great for small things, but as soon as we get more than two levels of indentation it's starting to take a toll on my mental RAM.

Maybe something that resembles VC slots?

modal_classes = ClassVariants.build(
  base: "inline-flex items-center rounded",
  variants: {
    color: {
      red:  "bg-red-600",
      blue: "bg-blue-600",
    },
    border: "border"
  }
) do |classes|
  classes.title "text-sm" # I'm not sure this is possible

  # If not, maybe this?
  classes.title do
    "text-sm"
  end
end

I'm open to suggestions for this.

How about we start with PRs for the small ones like 1. and 3. and open new issues for the next ones to track and start working after?

javierav · 2024-10-06T16:23:11Z

Thanks Adrian for the quick response!

Regarding your proposal in point 2, the way to specify the conditions on which these classes should apply, whether they are variants or composite variants, is missing, although it gives me the opportunity to comment on a point that I had left out of the previous list but that I am using right now in my application: using a code block to define the configuration instead of a hash.

ClassVariants.build do
  base <<~BASE
    inline-flex items-center justify-center gap-1 rounded-md font-semibold disabled:cursor-not-allowed disabled:opacity-30
  BASE

  variants do
    color do
      white "bg-white text-gray-900"
    end
    size do
      xs "text-xs px-2 py-1"
      sm "text-sm px-2 py-1"
      md "text-base px-2.5 py-1.5"
    end
    solid "shadow-sm"
    uppercase "uppercase"
  end

  defaults do
    border true
    color :white
    size :md
    solid true
  end
end

If we adapt it to use the new slots, it could look something like this:

ClassVariants.build do
  base do
    title "..."
    body "..."
  end

  variants do
    color do
      white do
        title "..."
        body "..."
      end
      black do
        title "..."
        body "..."
      end
    end
  end
end

Although this leads me to reintroduce a possibility that I had not initially contemplated in the suggested changes: unifying the variants and the compound variants into a single configuration:

ClassVariants.build do
  base do
    container "rounded py-3 px-5 mb-4",
    title "font-bold mb-1"
  end

  variant type: :outlined do
    container "border"
  end

  variant type: :outlined, severity: :error do
    container "border-red-700 dark:border-red-500"
    title "text-red-700 dark:text-red-500"
    message "text-red-600 dark:text-red-500"
  end

  variant type: :outlined, severity: :success do
    container "border-green-700 dark:border-green-500"
    title "text-green-700 dark:text-green-500"
    message "text-green-600 dark:text-green-500"
  end

  variant type: :filled, severity: :error do
    container "border-red-700 dark:border-red-500"
    title "text-red-700 dark:text-red-500"
    message "text-red-600 dark:text-red-500"
  end

  variant type: :filled, severity: :success do
    container "bg-green-100 dark:bg-green-800"
    title "text-green-900 dark:text-green-50"
    message "text-green-700 dark:text-green-200"
  end
end

Thus, for the options type: outlined, severity: :success and slot container it will return the classes rounded py-3 px-5 mb-4 border border-green-700 dark:border-green-500.

And if we don't need to work with slots because it is a simple component, it could look like this:

ClassVariants.build do
  base "inline-block w-2.5 h-2.5 rounded-md"

  variant color: :green, class: "bg-green-300 border-green-400"
  variant color: :yellow, class: "bg-amber-300 border-amber-400"
  variant color: :red, class: "bg-red-300 border-red-400"
end

Or

ClassVariants.build do
  base "inline-block w-2.5 h-2.5 rounded-md"

  variant color: :green do
    "bg-green-300 border-green-400"
  end

  variant color: :yellow do
    "bg-amber-300 border-amber-400"
  end

  variant color: :red do
    "bg-red-300 border-red-400"
  end
end

Or implement for these simple cases the variants method in plural as in previous examples:

ClassVariants.build do
  base "inline-block w-2.5 h-2.5 rounded-md"

  variants do
    color do
      green "bg-green-300 border-green-400"
      yellow "bg-amber-300 border-amber-400"
      red "bg-red-300 border-red-400"
    end
  end
end

And leave the variant method for the union of simple and compound variables and the variants method for when you want to work with something simpler.

How about we start with PRs for the small ones like 1. and 3. and open new issues for the next ones to track and start working after?

On this subject, in my humble opinion, I have the feeling that the way in which we define the structure of point 2 will condition the implementation in code of everything else, especially because of the issue of slots, so I would say that until we have a structure with which we feel comfortable we should not start working on the rest.

adrianthedev · 2024-10-06T23:17:55Z

Thanks for thinking this through.

Regarding simple, non-slot scenarios, I presume we'll still keep the current (hash) configuration.
Otherwise, this simple API looks great to me.

ClassVariants.build do
  base "inline-block w-2.5 h-2.5 rounded-md"

  variant color: :green, class: "bg-green-300 border-green-400"
  variant color: :yellow, class: "bg-amber-300 border-amber-400"
  variant color: :red, class: "bg-red-300 border-red-400"
end

I wouldn't open up block just to pass strings inside if we could do it using a kw argument.

Now, if we use class for simple variants, shouldn't we use class: for slots as well to keep it consistent?

# instead of
  variant type: :outlined, severity: :error do
    container "border-red-700 dark:border-red-500"
    title "text-red-700 dark:text-red-500"
    message "text-red-600 dark:text-red-500"
  end

# we add `class:`
  variant type: :outlined, severity: :error do
    container class: "border-red-700 dark:border-red-500"
    title class: "text-red-700 dark:text-red-500"
    message class: "text-red-600 dark:text-red-500"
  end

Regarding composed variants (type: :outlined, severity: :error), do we also keep the split behavior? I guess so, right?

# this should work
  variant type: :outlined, severity: :error do
    container class: "border border-red-700 dark:border-red-500"
    title class: "border text-red-700 dark:text-red-500"
    message class: "border text-red-600 dark:text-red-500"
  end

# this should work and give the same result, right?
  variant type: :outlined do
    container class: "border"
    title class: "border"
    message class: "border"
  end

  variant severity: :error do
    container class: "border-red-700 dark:border-red-500"
    title class: "text-red-700 dark:text-red-500"
    message class: "text-red-600 dark:text-red-500"
  end

javierav · 2024-10-07T10:03:53Z

I wouldn't open up block just to pass strings inside if we could do it using a kw argument.

I think the same too.

Now, if we use class for simple variants, shouldn't we use class: for slots as well to keep it consistent?

# instead of
  variant type: :outlined, severity: :error do
    container "border-red-700 dark:border-red-500"
    title "text-red-700 dark:text-red-500"
    message "text-red-600 dark:text-red-500"
  end

# we add `class:`
  variant type: :outlined, severity: :error do
    container class: "border-red-700 dark:border-red-500"
    title class: "text-red-700 dark:text-red-500"
    message class: "text-red-600 dark:text-red-500"
  end

In this case I see it as redundant to have to pass the class as a kw argument when it is the only parameter you are going to have to pass. Would it make sense to do something like this?

variant type: :outlined, severity: :error do
  slot :container, class: "border-red-700 dark:border-red-500"
  slot :title, class: "text-red-700 dark:text-red-500"
  slot :message, class: "text-red-600 dark:text-red-500"
end

By the way, we are making it clear that what we are defining is a slot and the class it should have.

Regarding composed variants (type: :outlined, severity: :error), do we also keep the split behavior? I guess so, right?

# this should work
  variant type: :outlined, severity: :error do
    container class: "border border-red-700 dark:border-red-500"
    title class: "border text-red-700 dark:text-red-500"
    message class: "border text-red-600 dark:text-red-500"
  end

# this should work and give the same result, right?
  variant type: :outlined do
    container class: "border"
    title class: "border"
    message class: "border"
  end

  variant severity: :error do
    container class: "border-red-700 dark:border-red-500"
    title class: "text-red-700 dark:text-red-500"
    message class: "text-red-600 dark:text-red-500"
  end

In this example you give, if several variant definitions meet the criteria, their classes must be mixed in the final result.

adrianthedev · 2024-10-07T21:47:32Z

Wow!
I think I like this API the most:

variant type: :outlined, severity: :error do
  slot :container, class: "border-red-700 dark:border-red-500"
  slot :title, class: "text-red-700 dark:text-red-500"
  slot :message, class: "text-red-600 dark:text-red-500"
end

It's because:

we are usign the method name as the thing that we are defining (variant, slot)
It's going to be easier to implement
it's easier to understand
we get to keep class: because we use it in the basic implementation (below)

ClassVariants.build do
  base "inline-block w-2.5 h-2.5 rounded-md"

  variant color: :green, class: "bg-green-300 border-green-400"
  variant color: :yellow, class: "bg-amber-300 border-amber-400"
  variant color: :red, class: "bg-red-300 border-red-400"
end

adrianthedev · 2024-10-07T21:48:00Z

Awesome work @javierav!

javierav · 2024-10-08T09:00:54Z

@adrianthedev this is how the configuration possibilities would look after what we have been talking about so far:

ClassVariants.build(
  "...", # positional arg, print deprecation warning
  base: "...",
  variants: {
    color: {
      red: "...",
      black: "..."
    },
    type: {
      button: "...",
      link: "..."
    }
  },
  compoundVariants: [], # supported? print deprecation warning?
  defaults: {
    color: :red,
    type: :button
  }
) do
  # base without slots
  base "..." # override hash definition? merge with it?

  # base with slots
  base do
    slot :head, class: "..."
    slot :body, class: "..."
  end

  # variant without slots
  variant color: :red, class: "..."

  # variant with slots
  variant color: :red do
    slot :head, class: "..."
    slot :body, class: "..."
  end

  # compound variant without slots
  variant type: :button, color: :red, class: "..."

  # compound variant with slots
  variant type: :button, color: :red do
    slot :head, class: "..."
    slot :body, class: "..."
  end

  # option 1 (my favorite)
  defaults color: :red, type: :button

  # option 2
  defaults do
    color :red
    type :button
  end
end

Decisions must be made based on this structure:

Are we going to allow using a hash and a code block at the same time? If the code block is used, does the hash configuration get discarded or are they mixed (this can be a bit complex to implement)?
Are we going to support composite variants in the hash as we have done until now or do we consider that together with the advanced configuration slots and therefore it is defined in the code block?
Are we going to allow setting default values in the code block or will it only be possible to do so through the hash? (option 1, option 2 or none)

I don't know if it makes sense to leave the hash option or go directly to the code block option that allows you to do more things in a more elegant way.

javierav · 2024-10-08T09:35:58Z

We also have to make decisions when calling the render method:

alert_classes = ClassVariants.build ...

# render default slot with default variants
# but whats happen if slots are used in definition?
# at this point we don't know which slot render...
# throw error? render empty string?
alert_classes.render

# render default slot with custom variants
alert_classes.render(color: :red)

# render slot with defaults variants
alert_classes.render(:body)

# render slot with custom variants
alert_classes.render(:body, color: :red)

# if slot not exist, throw error? return empty classes?
alert_classes.render(:non_existent_slot, color: :red)

# render default slot with custom class (will be merged)
alert_classes.render(class: "...")

# render slot with custom class (will be merged)
alert_classes.render(:body, class: "...")

What is clear to me is that all the previous calculations must be done at the time of definition with build (now it is done when calling render), since if for the same component we are going to call render several times (one for each slot), the calculation must already be done for it to be efficient.

We also need to define how to configure Tailwind Merge. My proposal:

ClassVariants.configure do |config|
  config.tw_merge = true # by default is false
  config.tw_merge_config = { ... } # by default is nil
end

If the option to use Tailwind Merge is not enabled, the library will not be initialized, so we avoid directly depending on that gem.

adrianthedev · 2024-10-14T14:27:36Z

Thanks for this summarization @javierav!

For simplicity sake (if it's indeed simpler), I would not allow both hash and block configuration. Only one at a time
If it's not that difficult I would support compoundVariants in the hash. It's readable enough and I could see the case in wanting to use the "simple" hash version.
I'd do option 1. Seems simpler. There's also the option 3 where we set the default in the declaration (code below), but I'm not sold on it. You pick!
Maybe we can compute the classes and memoize it on render?
The render proposal looks good.
The Tailwind Merge proposal is good as well!

# declaring a default option
variant color: :red, class: "...", default: true

# memoization somewhere in the `render` method

def render
  return @result if @result.present?
  # do things to figure out the classes
  result[:default] = "bg-red-500..."
  result[:some_slot] = "border..."

  @result = result
end

javierav · 2024-10-14T14:37:58Z

@adrianthedev Yeah! I'm going to start working on it!

javierav · 2024-10-18T08:03:37Z

@adrianthedev I think all the tasks have been completed. The PRs are in draft form because they will have to be updated as they are merged, as many of them are dependent on each other.

Deprecations Deprecate usage of positional arguments #12 Deprecate compoundVariants in favor of compound_variants #20
Slot support Add support for slots and advanced configuration via code block #15
Class mixing strategies Add support for post process classes #16
Override classes Allow passing additional classes when render #17
Helper to include in other classes Add helper module for defining variants #18

adrianthedev · 2024-10-21T17:57:18Z

@javierav. You are a machine 🔥 Thanks for working on these!

javierav · 2024-10-22T08:45:40Z

@adrianthedev The next steps are:

You: Merge PR Deprecate usage of positional arguments #12
Me: Rebase PR Deprecate compoundVariants in favor of compound_variants #20
You: Merge PR Deprecate compoundVariants in favor of compound_variants #20
You: Release gem version 0.0.8 (with all deprecations warnings)
(15..18).each do |i|
- Me: Rebase PR #{i}
- You: Merge PR #{i}
Me: create new PR with removals of all deprecations
You: Merge these PR
You: Release gem version 1.0.0

adrianthedev · 2024-10-22T09:01:26Z

Hey @javierav.
Would you like to do that?
I gave you write permissions for this repo.
I feel that you're very invested in it and your contributions are spot on!

javierav · 2024-10-24T10:27:15Z

All that's left to do now is update the readme to explain how all these changes work and release version 1.0.0. My English level isn't that good, how is yours @adrianthedev? 😆

adrianthedev · 2024-10-24T11:31:43Z

Yup. I'll take care of it @javierav.
Thanks for all your help! You did fantastic work!

adrianthedev · 2024-11-03T13:17:26Z

Continuing the conversation here
#22

adrianthedev closed this as completed Nov 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proposal for changes #9

Proposal for changes #9

javierav commented Oct 5, 2024

adrianthedev commented Oct 5, 2024 •

edited

Loading

javierav commented Oct 6, 2024

adrianthedev commented Oct 6, 2024

javierav commented Oct 7, 2024

adrianthedev commented Oct 7, 2024

adrianthedev commented Oct 7, 2024

javierav commented Oct 8, 2024 •

edited

Loading

javierav commented Oct 8, 2024

adrianthedev commented Oct 14, 2024

javierav commented Oct 14, 2024

javierav commented Oct 18, 2024

adrianthedev commented Oct 21, 2024

javierav commented Oct 22, 2024

adrianthedev commented Oct 22, 2024

javierav commented Oct 24, 2024

adrianthedev commented Oct 24, 2024

adrianthedev commented Nov 3, 2024

Proposal for changes #9

Proposal for changes #9

Comments

javierav commented Oct 5, 2024

1. Configuration structure

2. Slot support

3. Override classes

4. Class mixing strategies

5. Helper to include in other classes

adrianthedev commented Oct 5, 2024 • edited Loading

javierav commented Oct 6, 2024

adrianthedev commented Oct 6, 2024

javierav commented Oct 7, 2024

adrianthedev commented Oct 7, 2024

adrianthedev commented Oct 7, 2024

javierav commented Oct 8, 2024 • edited Loading

javierav commented Oct 8, 2024

adrianthedev commented Oct 14, 2024

javierav commented Oct 14, 2024

javierav commented Oct 18, 2024

adrianthedev commented Oct 21, 2024

javierav commented Oct 22, 2024

adrianthedev commented Oct 22, 2024

javierav commented Oct 24, 2024

adrianthedev commented Oct 24, 2024

adrianthedev commented Nov 3, 2024

adrianthedev commented Oct 5, 2024 •

edited

Loading

javierav commented Oct 8, 2024 •

edited

Loading