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

Tonyvanswet/ele 1346 UI panel rankingintro #26

Closed
wants to merge 6 commits into from

Conversation

tonypls
Copy link

@tonypls tonypls commented Nov 10, 2022

Started on ranking panel.

Realised that we need the translations and a solid way to shape the data.

I'd mostly like feedback around how we handle the fetching, caching and shaping of data.

Here's my logic around the process here:

Use react query solely for fetching and caching the data blobs returned by the apis, useGetState / useGetDetail will always mirror the last api response for the given time period we're interested in i.e. Month

Use the useMemo hook for calling functions that map that data from useGetState / useGetDetail based on the datetime we're interested in and the data shape that suits the function.

I tried loading all the logic into useQuery and yes it's possible to create more hooks to do the data shaping, I feel like we already have the benefit of react query (stale time etc) at the api level and useMemo makes things a bit more simple.

@Kongkille Kongkille mentioned this pull request Nov 14, 2022
Kongkille pushed a commit that referenced this pull request Nov 14, 2022
@Kongkille Kongkille mentioned this pull request Nov 14, 2022
@Kongkille
Copy link

Heyo @tonypls,

I've moved some things around on the map logic (#27) to get the colors blazingly fast. To do so, we separate the geometries from map data.

That is slightly different than this version, so to not disrupt your work, i've created a new PR #32 which follows up on this PR.

Overall I agree with your thoughts on when to useMemo, however, in this case I removed the useMemo because setting up a memo on a value that depends on the datetimeindex is probably not worth it due to the initialization costs. It works pretty fast now, if we want to speed it up we can do some other things, such as only rendering the top 10 results or something like that.

There may be some cases on using memo for derived data-states but otherwise we should consider when we're using memo whether it makes more sense to just send the data directly from the app-backend instead of computing it in the frontend.

@tonypls tonypls closed this Nov 15, 2022
tonypls added a commit that referenced this pull request Nov 17, 2022
* add locales

* add translation logic

* use translation hook in timecontroller

* Copied files from #26

* working prototype

* ensure timecontroller is on top

* ranking rows

* refined ranking panel

* remmove undefined

* Adds onClick handler

* Applies curly rule

* Update web/src/features/panels/ranking-panel/ZoneList.tsx

Co-authored-by: Mads Nedergaard <[email protected]>

* Creates and uses an InternalLink component that preserves state

* remove ref and update hover

Co-authored-by: tonypls <[email protected]>
Co-authored-by: Mads Nedergaard <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants