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 card without chart #1117

Merged
merged 17 commits into from Apr 26, 2018

Conversation

Projects
None yet
9 participants
@c727
Contributor

c727 commented Apr 22, 2018

backend PR: home-assistant/home-assistant#14076

fix: #999
fix: #464

current:
current

PR:
unbenannt

@c727

This comment has been minimized.

Contributor

c727 commented Apr 22, 2018

We could also use these colored icons (with or without animation) but they are not 100% material https://www.amcharts.com/free-animated-svg-weather-icons/
icons

c727 added some commits Apr 22, 2018

@arsaboo

This comment has been minimized.

Contributor

arsaboo commented Apr 22, 2018

Super excited about this 👍

Wondering if user can configure whether to use the animated icons or the static ones for the weather card.

</div>
<div class="attribute flex">
<div class="icon-container">
<iron-icon icon="mdi:flag-variant"></iron-icon>

This comment has been minimized.

@arsaboo

arsaboo Apr 23, 2018

Contributor

mdi:weather-windy may be a better icon for wind_speed.

"humidity": "Humidity",
"visibility": "Visibility",
"wind_bearing": "[\"N\", \"NNE\", \"NE\", \"ENE\", \"E\", \"ESE\", \"SE\", \"SSE\", \"S\", \"SSW\", \"SW\", \"WSW\", \"W\", \"WNW\", \"NW\", \"NNW\", \"N\"]",

This comment has been minimized.

@c727

c727 Apr 23, 2018

Contributor

is this OK?

@balloob

This comment has been minimized.

Member

balloob commented Apr 23, 2018

The middlepart looks messy and cramped. The measurements are not aligned with icon/label. We need a better way.

@c727

This comment has been minimized.

Contributor

c727 commented Apr 23, 2018

what about this? suggestions are welcome
1
this

@c727

This comment has been minimized.

Contributor

c727 commented Apr 23, 2018

2
without text
3
without text2

btw: @arsaboo we can't change the icon set without using customize and the code for colored icons was already rejected

@arsaboo

This comment has been minimized.

Contributor

arsaboo commented Apr 23, 2018

I like no. 2.

@pepeEL

This comment has been minimized.

pepeEL commented Apr 23, 2018

For me good is no. 1 and no. 2

@balloob

This comment has been minimized.

Member

balloob commented Apr 23, 2018

The challenge with 3 is the lack of description. 2 is still chaotic. Every row has it's own column section. I prefer 1.

make sure btw that everything on the left is aligned. Look how the table starts further in than the icon and the icon starts further in than the header. It should all be starting at the same line.

@balloob

This comment has been minimized.

Member

balloob commented Apr 23, 2018

Thinking about it more, maybe the humidity, pressure etc are not important enough to be in the card. We can move them to the more info ?

@raccettura

This comment has been minimized.

Contributor

raccettura commented Apr 23, 2018

I'd say humidity is pretty important for a decent sized part of the world where heat index is a normal part of the forecast during warmer months. Pressure IMHO is not terribly important for most people.

@balloob

This comment has been minimized.

Member

balloob commented Apr 23, 2018

Fair point. So let's at least scrap pressure from the main view.

@NovapaX

This comment has been minimized.

Contributor

NovapaX commented Apr 23, 2018

Maybe limit the amount of forecast days shown in the card?
I'd say 5 or 7 would be enough, the rest can go in the more-info in a list-style instead of columns.

@c727

This comment has been minimized.

Contributor

c727 commented Apr 23, 2018

new design in first post, forecast has max. 7 days now

c727 added some commits Apr 23, 2018

@arsaboo

This comment has been minimized.

Contributor

arsaboo commented Apr 23, 2018

Looks good. Can we fix the alignment of the text on the right to be closer to your post here

@arsaboo

This comment has been minimized.

Contributor

arsaboo commented Apr 23, 2018

Also, for English, should we use the standard 3 letters for days (Mon, Tue, etc.) now that we have some space?

@c727

This comment has been minimized.

Contributor

c727 commented Apr 23, 2018

...this is German

@c727

This comment has been minimized.

Contributor

c727 commented Apr 24, 2018

please check if you agree with my changes in translation file

more-info

c727 added some commits Apr 24, 2018

@fanthos

This comment has been minimized.

Contributor

fanthos commented Apr 24, 2018

I like the new design mostly, but the table style does not work well with platform that has per-hour forecast, like OpenWeatherMap.
#375 #373
I think we can create a bar with weather condition, but needs more work to make it align with the chart.

@sgttrs

This comment has been minimized.

Contributor

sgttrs commented Apr 24, 2018

Maybe visibility can also be hidden?

@c727

This comment has been minimized.

Contributor

c727 commented Apr 24, 2018

Can we focus the discussions on major issues like fanthos posted, please

@c727 c727 referenced this pull request Apr 24, 2018

Closed

[help wanted] unify weather components #14071

2 of 5 tasks complete
@fanthos

This comment has been minimized.

Contributor

fanthos commented Apr 24, 2018

I think the per-hour works better in more cases. The daily forecast convert could be done in frontend if needed.

@fanthos

This comment has been minimized.

Contributor

fanthos commented Apr 24, 2018

Relative easy to replace Google Charts with Chart.js.
image
Code:
https://github.com/fanthos/home-assistant-polymer/tree/weather

@c727

This comment has been minimized.

Contributor

c727 commented Apr 24, 2018

IMO if you have both, temp and tempLow, a graph makes no sense
yahoo and BR have this for example

@fanthos

This comment has been minimized.

Contributor

fanthos commented Apr 24, 2018

You are right.
But I think we better keep existing data, or provide feature like fallback to chart.
After some search, many API providers has free hourly forecast for developers.
https://www.wunderground.com/weather/api/d/pricing.html

@c727 c727 referenced this pull request Apr 24, 2018

Merged

Modify weather components for "new" frontend card #14076

2 of 2 tasks complete
@c727

This comment has been minimized.

Contributor

c727 commented Apr 24, 2018

finished :)

final

@balloob balloob merged commit a983a5d into master Apr 26, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@balloob balloob deleted the weathercard branch Apr 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment