Custom libraries misconfigured

[mattpolito]

Is there a reason that "custom" libraries need to be under the "custom" library key?

They don't appear to work the same as the preconfigured libraries. For example, I created a custom library with the generator:

# config/initializers/rails_icons.rb

RailsIcons.configure do |config|
  config.libraries.merge!(
    {
      custom: {
        coolicons: {
          # path: "app/assets/svg/icons/coolicons/",
          default: {
            css: "size-6"
          }
        }
      }
    }
  )
end

Files added to app/assets/svg/icons/coolicons/classic/

I can then use the helper like so:

icon(:envelope, library: :coolicons, variant: :classic)

Great, now I want to modify to have a default variant and this is where things start to not work the same. When I change the initializer to:

# config/initializers/rails_icons.rb

RailsIcons.configure do |config|
  config.libraries.merge!(
    {
      custom: {
        coolicons: {
          # path: "app/assets/svg/icons/coolicons/",
          default: {
            css: "size-6"
          },
          default_variant: "classic"
        }
      }
    }
  )
end

I am unable to find the correct default variant attribute because of the way the custom libraries are looked up. The library itself is found fine with the custom_library method but that is not used when setting the default variant. So then the variant is not found.

After looking through the source I didn't really see a reason to mark custom libraries and 'custom' other than the possibility that there could be a naming collision with a pre-defined library.

So I tried just changing the config and this worked out really well with no changes to the codebase.

# config/initializers/rails_icons.rb

RailsIcons.configure do |config|
  config.libraries.merge!(
    {
      coolicons: {
        # path: "app/assets/svg/icons/coolicons/",
        default: {
          css: "size-6"
        },
        default_variant: "classic"
      }
    }
  )
end

Configuring this way makes the "custom" library work exactly as every other library as far as I can tell and now I can utilize the defaults

icon(:envelope, library: :coolicons)

If this makes sense to you, I propose a documentation change and then possibly the removal of the custom_library & custom_library? methods.

[eelcoj]

Thanks for raising this. I haven't personally needed custom icons in any apps for a long time so haven't stumbled upon this.

Dove a bit into this and it looks like some things are off here.

This correctly looks up the icons in the simple/classic directory, but the default: {css: "size-6" } is not found/applied:

RailsIcons.configure do |config|
  config.libraries.merge!(
    {
        simple: {
          default: {
            css: "size-6"
          },
          default_variant: "classic"
        }

    }
  )
end

For that to work the documented/generated code is needed:

RailsIcons.configure do |config|
  config.libraries.merge!(
    {
      custom: {
        simple: {
          # path: "app/assets/svg/icons/simple/",
          default: {
            css: "size-6"
          },
          default_variant: "classic"
        }

      }
    }
  )
end

But now the default_variant is indeed missing/not applied. That feature was added here: https://github.com/Rails-Designer/rails_icons/pull/71/files

So I'd say this is a bug in the way the default_variant is set up OR the default attributes (class, data, etc) are set up. That said, I prefer not to have to define a custom key in the configuration if not needed.

[mattpolito]

I agree. I originally expected the default variant to be declared like this:

RailsIcons.configure do |config|
  config.libraries.merge!(
    {
      custom: {
        simple: {
          # path: "app/assets/svg/icons/simple/",
          default: {
            css: "size-6",
            variant: "classic"
          }
        }
      }
    }
  )
end

Why have a defaults key if all the defaults are not going to go in it.

As far as the custom key not being needed. I am using it that way now with no issues and no code changes... so it's definitely not needed. The only thing I can think of is like I said before, a possibly naming collision which hopefully would be rare.