glanceapp / glance

A self-hosted dashboard that puts all your feeds in one place
GNU Affero General Public License v3.0
8.56k stars 297 forks source link

Add Dashboard Icons support #255

Closed 2q2code closed 6 days ago

2q2code commented 1 week ago
2q2code commented 1 week ago

I forked this from main, but when I went to do the PR, it said to use the latest release branch, so I'm not sure what to do about this, however, it builds, and has been tested, using the main branch. It does not have any negative effect on existing glance.yml configs (at least for my own config), but given the simplicity of the modification, it should have no bad effects whatsoever.

2q2code commented 1 week ago

Having a few minutes to think, I would probably rename the constant LocalFile and the reference to RemoteResourceIcon to something like URI and IconShortCode or something. Meh.

2q2code commented 1 week ago

Made the noted changes above, and also added support to use either SVG or PNG version of DashBoardIcons. It makes more sense now so I can stop touching it. :sweat_smile:

2q2code commented 1 week ago

The extended notation mechanism is noted in code comments in fields.go.

svilenmarkov commented 1 week ago

Hey, thanks for contributing!

To avoid having to duplicate the same logic in future widgets that also have user provided icons, I think what might make more sense is to have a custom type that keeps the logic and data in one place:

type CustomIcon struct {
    URL        string
    IsFlatIcon bool
}

func (i *CustomIcon) UnmarshalYAML(node *yaml.Node) error {
    var value string
    if err := node.Decode(&value); err != nil {
        return err
    }

    prefix, icon, found := strings.Cut(value, ":")
    if !found {
        i.URL = value
        return nil
    }

    // the rest of the code from toIconURIIfPrefixed

    return nil
}

This could then easily be used in structs as:

struct {
    Icon CustomIcon `yaml:"icon"`
}

And wouldn't need iterating and applying things manually in each widget like this anymore:

link.Icon, link.IconSource = toIconURIIfPrefixed(link.Icon)
link.IsSimpleIcon = link.IconSource == SimpleIcon

(though would require tweaking the current templates a little)

I'll get that done and merged by the end of the week unless you'd like to make this change yourself.

2q2code commented 1 week ago

Your approach sounds solid. I've no more time to give this week, so I'll leave it to you. I did have some other ideas around the flat icons: allowing this to be user set or forced off if already on due to simple icon syntax, and also, an idea not fully formed yet, is the use of the colour values on simple icons' website to tint the icons their trademark colours if desired, or another colour entirely. Then the flag would become monochrome perhaps instead of flat. I have other ideas but another time. Have a good weekend. Nice to meet you!

svilenmarkov commented 6 days ago

No worries, got it sorted out. It should be much easier to make changes to the icons implementation now, should you want to try realizing those ideas later down the line. Thanks again for contributing and have a nice weekend!