-
Notifications
You must be signed in to change notification settings - Fork 279
feat(WEATHER): localized number values and support "pressure" key #666
Conversation
mikyjazz
commented
Mar 11, 2021
- added filters to localize numeric values according to locale
- changes in CLIMATE tile
- added function filterNumber in $scope to call from anonymous functions (existing function numberFilter does not use locale)
- minor changes to weather tile
version alignment
align changes in master
I have a test tile that renders like this before (master) and after (this change): Here is the tile's code: {
title: 'Home',
position: [6, 6],
height: 2.5, width: 2,
// classes: ['-compact'], // enable this if you want a little square tile (1x1)
type: TYPES.WEATHER,
id: 'weather.home',
state: function () {return 'Clear, night'}, // https://github.com/resoai/TileBoard/wiki/Anonymous-functions
icon: 'clear-night',
icons: {
'clear-night': 'nt-clear',
'cloudy': 'cloudy',
'exceptional': 'sunny',
'fog': 'fog',
'hail': 'sleet',
'lightning': 'chancestorms',
'lightning-rainy': 'tstorms',
'partlycloudy': 'partlycloudy',
'pouring': 'rain',
'rainy': 'chancerain',
"snowy": 'snow',
'snowy-rainy': 'sleet',
'sunny': 'sunny',
'windy': 'hazy',
'windy-variant': 'flurries'
},
states: {
"clear-night": "Clear, night",
"cloudy": "Cloudy",
"exceptional": "Exceptional",
"fog": "Fog",
"hail": "Hail",
"lightning": "Lightning",
"lightning-rainy": "Lightning, rainy",
"partlycloudy": "Partly cloudy",
"pouring": "Pouring",
"rainy": "Rainy",
"snowy": "Snowy",
"snowy-rainy": "Snowy, rainy",
"sunny": "Sunny",
"windy": "Windy",
"windy-variant": "Windy"
},
fields: { // most of that fields are optional
summary: '&sensor.dark_sky_summary.state',
temperature: '&sensor.dark_sky_temperature.state',
temperatureUnit: '&sensor.dark_sky_temperature.attributes.unit_of_measurement',
windSpeed: '&sensor.dark_sky_wind_speed.state',
windSpeedUnit: '&sensor.dark_sky_wind_speed.attributes.unit_of_measurement',
humidity: '&sensor.dark_sky_humidity.state',
humidityUnit: '&sensor.dark_sky_humidity.attributes.unit_of_measurement',
list: [
// custom line
'Feels like '
+ '&sensor.dark_sky_apparent_temperature.state'
+ '&sensor.dark_sky_apparent_temperature.attributes.unit_of_measurement',
// another custom line
'Pressure '
+ '&sensor.dark_sky_pressure.state'
+ '&sensor.dark_sky_pressure.attributes.unit_of_measurement',
// yet another custom line
'&sensor.dark_sky_precip_probability.state'
+ '&sensor.dark_sky_precip_probability.attributes.unit_of_measurement'
+ ' chance of rain',
],
},
}, |
It looks like it actually looks correct with your changes. The only thing is that the windSpeed icon still has some negative margin applied. And of course, all the sensor values are not correct but that is expected as I don't have that sensor in HA. |
fixed wrong check
for the pressure it is right to use the appropriate field with the icon I added instead of putting it in the list (also in the example) |
ALWAYS a space must be before units |
please remove those changes |
"The International System of Units (SI) prescribes inserting a space between a number and a unit of measurement (being regarded as a multiplication sign) but never between a prefix and a base unit; a space (or a multiplication dot) should also be used between units in compound units." |
Please find any weather website that puts space before the degrees (celsius) symbol. |
not interested in what doing others... SI is law... |
We're not making a government approved application here. If there is widely applied convention that is not in line with the "law" then we should go for the convention. I'm gonna look more into it tomorrow but it doesn't seem right what you are suggesting (at least for the degrees symbol). |
And I'm 100% sure that you don't put space before the percentage sign. Nobody writes |
While ISO 31-0 dictates that there must be a non-breaking space between value and a symbol, all of the style guides that I've come across are against it. I am always driven by what actually looks and feels right and to me personally an absence of space looks more natural. |
Very well, the project is yours and you can usually decide what is right or best to do. For my part, coming from the academic world, it is obviously impossible to privilege the aesthetic aspect to the detriment of the rules. Go ahead and change the spacing, I'll continue with my fork ... Just to remind you, I initially corrected the examples where "C" was reported in the temperature unit instead of "°C". |
scripts/directives/headerItem.html
Outdated
@@ -23,7 +23,7 @@ | |||
</div> | |||
|
|||
<div class="header-weather--temperature" ng-if="item.fields.temperature"> | |||
<span ng-bind="getWeatherField('temperature', item, {})"></span> | |||
<span ng-bind="(getWeatherField('temperature', item, {}) | number:1)"></span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it actually make sense to show one decimal place for the outside temperature? I would argue that it's not because those numbers are never precise enough for that to matter. And again, I could give other weather forecasts as an example against it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In any way, this is not blocking the review, just wanted to know your opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a sensible question, all the services we normally get information from provide a decimal as precision, then it may or may not make sense to show it in the UI, I like to see it ... It would probably be nice to have the ability to configure this thing with a entry in config.json ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'd prefer not to see it as it would make the brain "ingest" the number more easily without the decimal digits but will ignore it for now.
Thanks! (Note that these changes didn't seem to have anything to do with the CLIMATE tile so I changed the summary accordingly). |
Yes that's right, there were other changes on climate that I excluded to make the pull request only cover one topic. At this point we have a problem, tell me how we should proceed ... In my fork there will always be the space between numbers and units and the decimal when "I like" ... What do I do? Would I do pull of other changes (but then you'll need to change you those things from you) or our "partnership" ends here? |
I also have a "custom" branch with my local changes that don't belong upstream (here) because those are either not mature or just too specific. That shouldn't pose much of a problem for creating PRs as long as those PRs don't modify the exact code that you have customized. You can develop the feature on your custom branch. Once you are ready to submit a PR, you can for example commit it on your custom branch, then create a new fresh branch based on upstream Once the PR is merged you can switch back to your custom branch and rebase it on upstream's master. There will likely be conflicts if there were some changes made during code review but should be easy to resolve (just pick the upstream side). (Alternatively, you could even avoid committing the initial version of your changes on your custom branch to avoid conflicts later, during rebase.) |
I know that you are right and I very much value the time you are spending to push this project forward. I have no scientific or even programming background, I'm an accountant and most of the time whatever I do is dictated solely but a gut feeling. |