-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
POC: Rollup view #5719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
POC: Rollup view #5719
Conversation
|
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.
I think this is good to evaluate, though querying for IDs alone multiple times per dashboard is probably unnecessary and we could utilize Sites Cache to retrieve them.
A bit weird that unescaped URI like rollup:3e6b15a0-64a3-495b-9b0d-7303562cea77
shows a blank page, not a dashboard nor 404 even.
This also bothered me slightly about this implementation although it's good enough for preliminary testing. A couple thoughts:
Zooming out a bit, the solutions seem quite heavy and not sure if proportional to the size of the problem. Overall I agree it's not optimal as is. |
Yup, good point, I only thought of that as a quick PoC.
That sounds unnecessary indeed.
Asynchronously - for sure. Perhaps we should look at scroll depth data to understand whether we should load everything at the start or resume when in viewport etc, similar to how it's done currently. Keeping the common context loaded once for all is definitely desirable. |
Yeah, that's because the React router expects a Fixing this would involve passing |
Thanks @RobertJoonas! Sounds reasonable |
Changes
This PR is a proof of concept that aims to uncover possible issues/limitations querying clickhouse for rollup sites. It does not touch any database fields yet, but allows viewing the dashboard of multiple sites at once. To be reverted as soon as we get the necessary insights.
Tests
Changelog
Documentation
Dark mode