Add dashboard widget - Githubissues

nextcloud / weather

⛅️ Weather app for Nextcloud

GNU Affero General Public License v3.0

47 stars 27 forks source link

Add dashboard widget #76

Closed berdosi closed 4 years ago

berdosi commented 5 years ago

This request implements a very basic Weather widget for the Nextcloud Dashboard: it shows the most important weather data for the currently selected home city.

screenshot-190811-233801

nerzhul commented 5 years ago

Great job. Is this ready for a merge ?

berdosi commented 5 years ago

Now that you mention it, right, it isn't:

The widget doesn't report errors.
There's a leftover console.info() in the JavaScript part; I'll lint it through for more findings.

I'll comment again once these are sorted out, probably in the coming days.

berdosi commented 5 years ago

Hello again! I consider it ready for a merge now.

e-alfred commented 4 years ago

@nerzhul @berdosi Now that all issues are fixed, do you think we can merge this request into the main repository?

nerzhul commented 4 years ago

@e-alfred can you verify it's valid against current master and test it ? code looks good to me currently

e-alfred commented 4 years ago

@berdosi I think the error handling could be improved for a better user experience:

Show an explicit message that no city is defined in the Weather app
Show an explicit message if no home city is defined in the Weather app

Maybe in a future improvement additional cities/towns can be added than just the home city.

e-alfred commented 4 years ago

@nerzhul It works well with the latest master branch, I think we could merge it.

e-alfred commented 4 years ago

@nerzhul I published a new release on the release page, please publish it to the Nextcloud App Store as soon as possible.

nerzhul commented 4 years ago

i'll do it

nerzhul commented 4 years ago

@e-alfred can you tag as 1.7.0, it's a new feature not a patch, thanks :)

e-alfred commented 4 years ago

I uploaded a new release as 0.1.7, please publish it to the App Store.

e-alfred commented 4 years ago

@berdosi It seems like the old Dashboard was deprecated and a new one is implemented into the core: https://github.com/nextcloud/server/issues/20930

berdosi commented 4 years ago

Thanks for the heads up! I had a quick look at the PR implementing the widget in Talk ( https://github.com/nextcloud/spreed/pull/3890 ), and it doesn't look like we can share a lot of code (maybe except for the widget's template, that ain't a lot).

nerzhul commented 4 years ago

it's nice if we can be up to date, nice job :)

e-alfred commented 4 years ago

Nextcloud server now has a weather dashboard app created by @eneiluj. Maybe we can collaborate on integrating both apps better in the future?

berdosi commented 4 years ago

Indeed, I followed this PR quite closely, maybe not close enough, though:) → https://github.com/nextcloud/server/pull/22124

from widget's point of view, the one-line status shows pretty much the same info as current (#76) did, but in a sleeker way.
currently clicking on the new widget brings us to the weather service - it may be good to be able to hook into that :)
reusing the same backend would be good as two weather services within Nextcloud may be one too many :) Currently that one is polling https://api.met.no/weatherapi/locationforecast/2.0/documentation#!/data/get_compact which doesn't show the same detail that the Weather app does – the "complete" endpoint seems to be better for our purposes.