victrme / Bonjourr

Minimalist & lightweight startpage inspired by iOS
https://bonjourr.fr
GNU General Public License v3.0
932 stars 107 forks source link

Add icon pack support for Weather component #370

Closed driedpampas closed 4 months ago

driedpampas commented 4 months ago

I got a cute idea and here is the result. I also forked and prepared an icon pack (material-weather-icons) already.

And I might have added myself to the Romanian translators, but it feels like I'm being greedy, so if needed it'll go poof

changes:

driedpampas commented 4 months ago

It occurred to me that we could iterate over folders and create a dynamic pack list which wouldn't require manually messing with the html but idk how to feel about it. (basically more feedback needed)

driedpampas commented 4 months ago

Also an unrelated question: Is the dev branch for the next major version or a staging spot for the next release's features (major or minor)

Also also, I saw the issue about the downloading backgrounds and i kinda wanna try my hand at it (mostly because it annoys me - i download way too many backgrounds from unsplash)

victrme commented 4 months ago

Nice @driedpampas ! This is a very interesting idea 🤔

We are super strict about the features that we add, which means I cannot accept your PR as it is. There are a few things to note:

This feature is a lot trickier than it seems. It might be easier to simply add a CSS class for different weather icons, and instruct users on how to add a different background-image for each icons using the custom CSS feature.

Having said all that, you can make a different PR on master to add your translation credits to the README (you deserve it), and the bun.lockb why not.

dev is also a staging branch, this is where things are sure to be added and should be working for the next version. If you want to try adding the download icon, you can create a different branch and a different PR. There is no guarantee that we will accept your changes however !

victrme commented 4 months ago

It might be easier to simply add a CSS class for different weather icons

You can also try that on a different branch if you want. If you manage to make it work, this will be included in the next major update !

Todo:

driedpampas commented 4 months ago

Thanks for the feedback and the todo list and I will make a separate pr for the tl credits and the .gitignore. I was expecting some of it, so I'll see if i can whip up something more robust but as you noted it will be quite tricky.