search

ajnart / homarr

Customizable browser's home page to interact with your homeserver's Docker containers (e.g. Sonarr/Radarr)
https://homarr.dev
MIT License
5.48k stars 253 forks source link

feat: show clock on weather widget (#1940) #1990

Closed pablodre closed 2 months ago

pablodre commented 3 months ago

Hi! Its my first issue! :)

Category

Feature

Overview

I add an option to show a clock in the weather widget with the time zone of the weather location

Issue Number (if applicable)

https://github.com/ajnart/homarr/issues/1940

New Vars (if applicable)

If you've added any new build scripts, environmental variables, config file options, dependency please outline here.

Screenshot (if applicable)

image image

SeDemal commented 2 months ago

Hi there, Thank you very much for contributing. First, I like the idea a lot, especially since it can be synced with the timezone automatically. I do have 1 concern and 1 opinion: Concern: doing this might set us on a path for requests about adding more feature of the original clock to this widget. Cluttering it and doubling any modification done to the first. Opinion: (opinion meaning that's a me thing, you're free to see my point or stand your idea) It kind of looks like it doesn't belong there, not that it shouldn't at all, but that it's placed wrong. Maybe because in some places it's taking a lot of space, even though it's supposed to be the weather widget.

ajnart commented 2 months ago

@pablodre Did you see @Tagaishi 's comment ?

pablodre commented 2 months ago

@Tagaishi @ajnart Really, this is a small feature that people who don't want to use the clock widget will love (like me), because they only need a simple clock.

It's just a clock option in a weather widget. All the future new advance features only should be available in the clock widget

But this is just my personal opinion. You are free to suggest a design change or close the PR

🙂

Meierschlumpf commented 2 months ago

Hey, thanks for the effort. From my point of view, there is no added value for the majority of our users, which is why I don't want to add this feature to Homarr. Maybe in the future there will be ways to achieve similar functionalities, but in my opinion it doesn't make sense like this. Thanks anyway, I'll close the pull request, hope that's okay for you.

pablodre commented 2 months ago

Hey, thanks for the effort. From my point of view, there is no added value for the majority of our users, which is why I don't want to add this feature to Homarr. Maybe in the future there will be ways to achieve similar functionalities, but in my opinion it doesn't make sense like this. Thanks anyway, I'll close the pull request, hope that's okay for you.

Not problem at all BUT the issue is still open: https://github.com/ajnart/homarr/issues/1940

I develop a request feature for an issue with more than 1 moth old, If "there is no added value for the majority of our users" the issue should be closed (or commented) for that reason, not closing a PR afther put effort in thar. Lucky us it is a small effort, but i think this is not the way...

SeDemal commented 2 months ago

We don't want to belittle any effort done by you or anyone in order to try and improve an open source project. We appreciate everyone of our contributors and a good 99% of PR's end up being accepted. We do appreciate your contribution as well, even if it wasn't accepted. The problem wasn't much with the execution, but rather the idea itself. We should have discussed and close the issue in question before, and that part is on us. I recommend you join our discord and simply ask before to make sure it doesn't happen again.

Meierschlumpf commented 2 months ago

Thanks and sorry, forgot to close the issue