Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weather module #961

Merged
merged 8 commits into from
Nov 27, 2020
Merged

Conversation

callemand
Copy link
Contributor

@callemand callemand commented Nov 17, 2020

Pull Request check-list

To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:

  • If your changes affects code, did your write the tests?
  • Are tests passing? (npm test on both front/server)
  • Is the linter passing? (npm run eslint on both front/server)
  • Did you run prettier? (npm run prettier on both front/server)
  • If you are adding a new features/services, did you run integration comparator? (npm run compare-translations on front)
  • If your changes modify the API (REST or Node.js), did you modify the API documentation? (Documentation is based on comments in code)

NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.

Description of change

Based on "#504" but use openweather api.

Ajout du nom de la maison servant de point de géolocalisation
Ajout du taux d’humidité et de la vitesse du vent
Ajout des prévisions météo horaires pour les 8 prochaines heures avec icônes et températures
Ajout des prévisions météo horaires pour les 7 prochains jours avec icônes et températures min et max

Screenshots

Screenshot 2020-11-17 at 09 34 36
Screenshot 2020-11-17 at 09 33 28

@callemand callemand changed the title Refactoring Weather module to display more informations and forecast Weather module Nov 17, 2020
@callemand callemand changed the title Weather module [WIP] Weather module Nov 17, 2020
@codecov
Copy link

codecov bot commented Nov 17, 2020

Codecov Report

Merging #961 (ea2e3d1) into master (abb17b3) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #961   +/-   ##
=======================================
  Coverage   94.51%   94.51%           
=======================================
  Files         485      485           
  Lines        6438     6438           
=======================================
  Hits         6085     6085           
  Misses        353      353           
Impacted Files Coverage Δ
server/models/dashboard.js 100.00% <ø> (ø)
server/services/openweather/index.js 92.59% <100.00%> (ø)
server/services/openweather/lib/formatResults.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update abb17b3...ea2e3d1. Read the comment docs.

@callemand callemand changed the title [WIP] Weather module Weather module Nov 17, 2020
Copy link
Contributor

@Pierre-Gilles Pierre-Gilles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi !

Thanks for your pull request, it looks really nice and I'm looking forward to merge this in Gladys Assistant 👍

Just one question, are we losing the big weather icon like it was before?

Maybe it could be nice to still have it when it "compressed" mode?

Screenshot 2020-11-20 at 15 32 29

Second question, it seems your PR is breaking the demo mode of Gladys Assistant (demo.gladysassistant.com)

Here is what I see in demo mode:

Screenshot 2020-11-20 at 15 42 55

To test the demo mode, you need to create a .env file in your "front" folder, with the value:

DEMO_MODE=true

And restart locally the frontend.

Let me know if you have any questions, and again thank you very much for your work here 🙂

@callemand
Copy link
Contributor Author

callemand commented Nov 20, 2020

Hi !

Thanks for your pull request, it looks really nice and I'm looking forward to merge this in Gladys Assistant 👍

Thanks 👍

Just one question, are we losing the big weather icon like it was before?

Yes, but it's a mistake, it's fixed now

Screenshot 2020-11-20 at 18 47 23

Maybe it could be nice to still have it when it "compressed" mode?
Yes, in compressed mode and all others mode

Screenshot 2020-11-20 at 18 41 14

Screenshot 2020-11-20 at 15 32 29

Second question, it seems your PR is breaking the demo mode of Gladys Assistant (demo.gladysassistant.com)

Here is what I see in demo mode:

Screenshot 2020-11-20 at 15 42 55

To test the demo mode, you need to create a .env file in your "front" folder, with the value:

DEMO_MODE=true

And restart locally the frontend.

Let me know if you have any questions, and again thank you very much for your work here 🙂

It's fixed too, I didn't know the debug mode on the front

Copy link
Contributor

@atrovato atrovato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope @Pierre-Gilles agree my comments.

front/src/actions/dashboard/boxes/weather.js Outdated Show resolved Hide resolved
front/src/components/boxs/weather/EditWeatherBox.jsx Outdated Show resolved Hide resolved
front/src/config/i18n/en.json Outdated Show resolved Hide resolved
front/src/components/boxs/weather/WeatherBox.jsx Outdated Show resolved Hide resolved
front/src/config/i18n/fr.json Outdated Show resolved Hide resolved
server/services/openweather/lib/formatResults.js Outdated Show resolved Hide resolved
@atrovato atrovato mentioned this pull request Nov 21, 2020
5 tasks
Copy link
Contributor

@Pierre-Gilles Pierre-Gilles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's some amazing work here 👏

Congrats for your great work, I'm merging this in Gladys right now!

@Pierre-Gilles Pierre-Gilles merged commit c2103b7 into GladysAssistant:master Nov 27, 2020
@Pierre-Gilles
Copy link
Contributor

@all-contributors please add @callemand for code

@allcontributors
Copy link
Contributor

@Pierre-Gilles

I've put up a pull request to add @callemand! 🎉

@Pierre-Gilles
Copy link
Contributor

@callemand could you maybe put a profile picture (or just an avatar, not necessarily your face) to your GitHub account? :)

So it looks better on the README:

Screenshot 2020-11-27 at 18 07 45

R6n0 pushed a commit to R6n0/Gladys that referenced this pull request Dec 2, 2020
…sistant#961)

* Refactoring Weather module to display more informations and forecast

* Prettier front

* Front eslint

* Front pretty

* Fix icon weather and debug mode

* Fix after atrovato review

Co-authored-by: Pierre-Gilles Leymarie <pierregilles.leymarie@gmail.com>
@callemand callemand deleted the weather_module branch July 11, 2023 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants