Zren / plasma-applet-simpleweather

https://store.kde.org/p/1287571/
4 stars 5 forks source link

Option to display Fahrenheit instead of Celsius #15

Closed nomodz4real closed 4 years ago

nomodz4real commented 4 years ago

When using wetter or BBC the temperature is displayed in Celsius. Is there any way to add an option to display Fahrenheit as well?

Zren commented 4 years ago

Anything the default weather widget supports, Simple Weather can. It's just a matter of implementing it.

Weather Forecast: GitHub | All Bugs | New Bug | Phabricator | Pull Requests | New PR (Repo: Plasma Addons)

Weather Forecast uses a C++ Util.temperatureToDisplayString(displayTemperatureUnit, tempHigh, reportTemperatureUnit, true) function. So I would need to recreate it in JavaScript most likely.

I'll take a stab at it the next time I've hacking simple weather.

alexdelorenzo commented 4 years ago

Just want to show support for this feature. Thanks for your awesome work, Zren!

Might take stab at it as well, if you're open to pull requests.

Zren commented 4 years ago

Patches are usually welcome, but probably not for this feature.

This one is a little complicated for a newcomer to do as the original weather widget uses C++ plasmoid.nativeInterface.temperatureUnitId for configuration. So you can't use the example code exactly. The patch I wrote does show how to convert it to use plasmoid.configuration.temperatureUnitId (like every other widget) but...

In my KDE Store widgets, I also avoid the "need to click Apply" intermediate step (which is usually by storing to configPage.cfg_temperatureUnitId first but not for the default weather widget). Since the example weather widget uses nativeInterface though, it instead calls the signal configurationChanged() to indicate changes were made (enabling the Apply button), then when you click Apply/Ok, it calls function saveConfig(), which uses plasmoid.nativeInterface.saveConfig({ ... }). Since we're not using that it'd be hard to simplify.

I've written the contents/ui/config/ConfigUnits.qml, just gotta implement the Units.temperatureToDisplayString() in a sane manner, then wrap all the format temperature calls with the new function.

Zren commented 4 years ago

Patches are usually welcome, but probably not for this feature.

This one is a little complicated for a newcomer to do as the original weather widget uses C++ plasmoid.nativeInterface.temperatureUnitId for configuration. So you can't use the example code exactly. The patch I wrote does show how to convert it to use plasmoid.configuration.temperatureUnitId (like every other widget) but...

In my KDE Store widgets, I also avoid the "need to click Apply" intermediate step (which is usually by storing to configPage.cfg_temperatureUnitId first but not for the default weather widget). Since the example weather widget uses nativeInterface though, it instead calls the signal configurationChanged() to indicate changes were made (enabling the Apply button), then when you click Apply/Ok, it calls function saveConfig(), which uses plasmoid.nativeInterface.saveConfig({ ... }). Since we're not using that it'd be hard to simplify.

I've written the contents/ui/config/ConfigUnits.qml, just gotta implement the Units.temperatureToDisplayString() in a sane manner, then wrap all the format temperature calls with the new function.

I also gotta handle the 0 = locale default...

Zren commented 4 years ago

Feature now in git master. Will probably release in a new version tonight or tomorrow.