search

irceline / aq-mobile-be

App for consulting air quality data in Belgium using the ionic framework for Android and iOS
Apache License 2.0
7 stars 8 forks source link

wrong styling and values longterm page #273

Closed opeeters closed 3 years ago

opeeters commented 3 years ago

On Android and iOS:

20210428_194338

Wrong value and styling (cf click model). The value should be the value of the last year (in this case value: 48). The font color should correspond with the relevant index color. On iOS the border thickness should also be reduced.

bieblebrox commented 3 years ago

Screenshot_20210430-124815_BelAir_v2

This is on latest build, only thing is that the bar chart colors seem to be different from the text (eg fairly good, shows red bar chart)

janschulte commented 3 years ago

Until now the big numbers are the current modelled values to every shown phenomenon. Now you want to change it to the value of the last year? I don't understand, why to show the value of the last year 3 times? If i am right, i would be much better to refactor the component, than overriding one value as happend...

janschulte commented 3 years ago

Hopefully everything is adjusted as whished.

opeeters commented 3 years ago

Looking good! Thanks @janschulte Some minor things in the browser version at least. I might come back to you if I cannot manage myself ;-)

opeeters commented 3 years ago

Screenshot_20210430-164019_BelAir_v2

opeeters commented 3 years ago

Until now the big numbers are the current modelled values to every shown phenomenon. Now you want to change it to the value of the last year? I don't understand, why to show the value of the last year 3 times? If i am right, i would be much better to refactor the component, than overriding one value as happend...

Cf click-model.. Not my idea.. The COM people ;-)

opeeters commented 3 years ago

I had another look at the click-model.. I was wrong.. different in the design. They put the EU-threshold above the graph:

Screen Shot 2021-04-30 at 16 56 30

but yea, twice the same value..

opeeters commented 3 years ago

After some internal discussion we decided to also add the WHO guideline values as text to the long-term tab:

Screen Shot 2021-04-30 at 23 41 25

@janschulte please check if my implementation was OK. Could you please still add the latest annual BelAQI to the view in detail pull-tab (cf click-model, but than as evaluation text - e.g. very good - and not a concentration):

Screen Shot 2021-04-30 at 23 35 29
opeeters commented 3 years ago

One more thing, the horizontal lines of the EU/WHO-limits in the graph are not correctly drawn. See e.g. (the red line indicates where the blue dashed line should be - see EU/WHO limit values):

Screen Shot 2021-04-30 at 23 41 25

@bieblebrox This is probably more a GUI-issue, no? In longterm-info-screen.component.html the values of euBenchMark and worldBenchMark are taken and they look correct to me.

janschulte commented 3 years ago

@janschulte please check if my implementation was OK. Could you please still add the latest annual BelAQI to the view in detail pull-tab (cf click-model, but than as evaluation text - e.g. very good - and not a concentration):

Looks good so far. I have done some refatoring in long-tem-data service to remove unused request for current values.

Additionally I add the adjustment to the details view with 2e2b86cb25d211272e44117b89d6b96e686a6bd7

janschulte commented 3 years ago

I hopefully doesn't miss anything in this Issue, otherwise please inform me again. I change the assigne to @bieblebrox for the threshold markers

bieblebrox commented 3 years ago

https://github.com/irceline/aq-mobile-be/commit/74804f001b4636ad9c5f82b749dc0f5479a50253