Skip to content
This repository has been archived by the owner on Nov 4, 2023. It is now read-only.

feat: locale option for dates and number formatting #644

Merged
merged 14 commits into from
Feb 21, 2021

Conversation

mikyjazz
Copy link
Contributor

verry sorry making trouble with git...
+/- all modifications included, please check...

rollup.config.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

@mikyjazz mikyjazz left a comment

Choose a reason for hiding this comment

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

Well done all changes... Really I don't manage git... Do I have to carry over the last changes you made "that do not belong to any branch" in my code or can the merge be done directly?

@rchl
Copy link
Collaborator

rchl commented Feb 21, 2021

You can git pull and you'll get my changes into your branch.

The change looks almost good to me. I'm playing with some remaining stuff:

  • use locale date for ChartJs defaults
  • add en-GB locale which is more useful for people who want non-US dates but still in english.

@mikyjazz
Copy link
Contributor Author

Ah ok, perfect, I wanted to understand if you had not done the merge because I had to carry out some operations.
In fact I hadn't thought about using chart.js otherwise I would have tried to look at the implementation ...
Maybe you could also enter some other default locale - I have no idea of the nationality of TileBoard users - for example be, cz, tk, dk ... cn?

@rchl
Copy link
Collaborator

rchl commented Feb 21, 2021

Maybe you could also enter some other default locale - I have no idea of the nationality of TileBoard users

I think the defaults you've selected are pretty good. We can add more later if needed.

@rchl
Copy link
Collaborator

rchl commented Feb 21, 2021

  • use locale date for ChartJs defaults

This would be a bit tricky actually because ChartJs uses moment to format dates but formats defined by angular-i18n are not always fully compatible with moment...

I'm good with those changes now and will merge if you don't see any issues.

@mikyjazz
Copy link
Contributor Author

Very well, I think we can proceed with the merge, I will also try to study the chartjs / moment problem ...

Copy link
Contributor Author

@mikyjazz mikyjazz left a comment

Choose a reason for hiding this comment

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

why relative path?

@rchl
Copy link
Collaborator

rchl commented Feb 21, 2021

why relative path?

If TileBoard is deployed to a non-root path like site.com/local/tileboard/ then absolute would most likely not work. Relative will.

@mikyjazz
Copy link
Contributor Author

why relative path?

If TileBoard is deployed to a non-root path like site.com/local/tileboard/ then absolute would most likely not work. Relative will.

right

@rchl rchl changed the title Set locale for dates and number format feat: locale option for dates and number formatting Feb 21, 2021
@rchl rchl merged commit ce236f2 into resoai:master Feb 21, 2021
@mikyjazz mikyjazz deleted the set_locale branch February 21, 2021 22:37
@rchl
Copy link
Collaborator

rchl commented Feb 21, 2021

Very well, I think we can proceed with the merge, I will also try to study the chartjs / moment problem ...

Here is what I was experimenting with: 4d0b775

This breaks graph tooltips though because the "medium": "d MMM y HH:mm:ss" format from locale files is not supported by moment.

Screenshot 2021-02-22 at 00 42 45

@mikyjazz
Copy link
Contributor Author

trying to investigate on chart, I created an HIstory item then I got this error:
Code -1 retrieved for http://192.168.1.245:8123/api/history/period/2021-02-17T19:05:04.995Z?end_time=2021-02-22T19:05:04.995Z&filter_entity_id=sensor.temperatura_cpu.
any idea ?

@rchl
Copy link
Collaborator

rchl commented Feb 22, 2021

You are probably hitting this issue #386

I've made a PR with a fix at home-assistant/core#43679 but it was confusingly rejected...

@mikyjazz
Copy link
Contributor Author

hi Rafal,
yes, this is certainly issue # 386.
I have read everything you wrote in the pull request on home assistant; in fact your solution could be approved as a temporary workaround but the problem should be fixed in a better way.
I looked a little in the Homeassistant code and saw that the History component is asynchronous and therefore also its initialization process. You should probably intervene in the process upstream of the async_setup call and check if CORS has been enabled in the configuration; in that case (and only in that case) it should be correctly enabled for the history resource (as for any others that follow that logic).
I don't know the logic behind HomeAssistant; if you set the flag to true (as in your proposed change) CORS is enabled to any address if nothing has been specified in configuration.yaml?
If this were the case, I think that the auditor was right not to accept your change ... As far as the change concerns the history, it is also true that through that passage you could possibly request sensitive information such as the shutdown and start times of a alarm...
Let's try to think of a solution by looking better in the Home Assistant code!
In the meantime I will move TileBoard to the HomeAssistant server to try out charts and dates!
I will tell you that I am proceeding in the localization of the states of the Weather component. I am also trying the OpenWeatherMap integration instead of DarkSky which - as you know - closes at the end of the year (so I read); I'll let you know how it goes.
See you soon
Michele

@rchl
Copy link
Collaborator

rchl commented Feb 23, 2021

yes, this is certainly issue # 386.
I have read everything you wrote in the pull request on home assistant; in fact your solution could be approved as a temporary workaround but the problem should be fixed in a better way.

We could discuss more on the HA issue but the last time I checked, I didn't see other API actually checking the origins whitelisted by the user.

@mikyjazz
Copy link
Contributor Author

OK ...
Please note that I am NOT a Python / Javascript / Angular programmer ... I am a really very old C / C ++ programmer!
For this reason I find it really difficult to deal with things like: blocks defined by indentation, untyped variables, weird ways of defining the visibility of methods and objects, implicit initialization processes, like init.py and bad things like that.
Now, but then, there is no other way to activate CORS on history than to install it on localhost?
Okay, let's go, I've been thinking about these kinds of changes, tell me what you think ... (at least on a theoretical level, it's obvious that the Home Assistant folks will never accept them ... Patience, I'll have to fork everything ... I have to have fun somehow ...)
Since History is an API, the idea is to move the definition of the HistoryPeriodView class to the api component ...

Therefore:

  1. move

class HistoryPeriodView(HomeAssistantView):
"""Handle history period requests."""

url = "/api/history/period"
name = "api:history:view-period"
extra_urls = ["/api/history/period/{datetime}"]

def __init__(self, filters, use_include_order):
    """Initialize the history period view."""
    self.filters = filters
    self.use_include_order = use_include_order

bla, bla, bla...

in api component (and relative initialization)

OR

create a derived class in api.py leaving this in place

class APIHistoryPeriodView(HistoryPeriodView):
"""View to handle Status requests."""

url = URL_API_HISTORYPERIOD     <-- DEFINE THIS IN CONST AS THE OTHERS AND REMOVE CORRESPONDING BASE CLASS DEFINITION
name = "api:status"

and then do the initialization at the right time!!!

async def async_setup(hass, config):
"""Register the API with the HTTP interface."""
hass.http.register_view(APIStatusView)
hass.http.register_view(APIEventStream)
hass.http.register_view(APIConfigView)
hass.http.register_view(APIDiscoveryView)
hass.http.register_view(APIStatesView)
hass.http.register_view(APIEntityStateView)
hass.http.register_view(APIEventListenersView)
hass.http.register_view(APIEventView)
hass.http.register_view(APIServicesView)
hass.http.register_view(APIDomainServicesView)
hass.http.register_view(APIComponentsView)
hass.http.register_view(APITemplateView)

ADDED!

hass.http.register_view(APIHistoryPeriodView)   OR  hass.http.register_view(HistoryPeriodView) IF MOVED INSTEAD MAKE DERIVED CLASS

if DATA_LOGGING in hass.data:
    hass.http.register_view(APIErrorLog)

return True

I have not checked what happens with regard to the dependencies also local of the HistoryPeriodView class or its references but I think it is an easily solvable problem.
In this way I believe the safety principle is respected and yet there should be no interference between the levels because api and history are both in components.

Finally, I didn't even try to figure out what the timing of the Home Assistant initialization workflow was (i.e. the reason for the current malfunction) because the project is too complex and there are no flow and class diagrams ... they have everything in mind, I don't!

See you

@rchl
Copy link
Collaborator

rchl commented Mar 3, 2021

Sorry, but at the moment I don't have time to go deep into this and try to understand how to fix this the best way.

I would prefer to discuss those things inside appropriate issues because otherwise, things get messy and hard to follow and find. This issue is about locale formatting so it's kinda out of place to discuss CORS issues here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants