Frontend Enhancements - 1 - #355
mamoutou-diarra wants to merge 4 commits into
Conversation
AlbertoSoutullo
left a comment
There was a problem hiding this comment.
Reviewing with AI. There are some points that I think they are not really super important right now but there is one that maybe it can be concerning. In the homepage, we fetch up to the latest 6 experiments. I think the preview in the homepage should be something we have in cache actually.
Good point, you mean in the browser cache? because now we're fetching them from mongo all the times and they're never cached because they are not static files like css or images. If we cache them in the browser, and a moment later we change the panel config in the backend, the user will still continue to see the old cache. He would need to go to its browzer settings and clear the cache which is not a good design in my opinion. There should be a way to do it properly, I'll dig deeper on my side |
PearsonWhite
left a comment
There was a problem hiding this comment.
Good incremental change.
radiken
left a comment
There was a problem hiding this comment.
LGTM with just one small optional improvement
AlbertoSoutullo
left a comment
There was a problem hiding this comment.
Can it be that right now we are caching partial results?
If the user navigates away while only one of several panel requests has completed, the cache contains a partial result. When the homepage is mounted again, that partial array is treated as a complete cache entry, so the remaining panels are never requested.
3f757b6 to
fb92440
Compare
Yes, good catch, I fixed it, now the key is a combination of experientID:PanelName, so each panel as a separate entry in the cache, so missed panels are guaranteed to be fetched when user returns to homepage |
AlbertoSoutullo
left a comment
There was a problem hiding this comment.
Last comment (I swear) we are doing 6 cards × 4 panels per card = 24 panel requests
I would reduce that to 6 total, because if not we are fetching an insane amount of data
Ok, in the home page we're showing only the last 6 experiments. I'll fetch one panel per experiment then. |
AlbertoSoutullo
left a comment
There was a problem hiding this comment.
The PR description says homepage thumbnails should have an “auto-rotating chart-panel preview.”. The current component fetches only one successful panel and renders it permanently. Which one are we sticking to?
Context
This PR polishes several parts of the UI and fixes some identified issues
Major changes
URLs