posthog
posthog copied to clipboard
PostHog
feat(insights): add existing insights to dashboard
Problem
Currently, users need to go the insights tab and individually add insights to their dashboards. In the current version of dashboards, there is only support to create a new insight and not add an existing one. This PR aims to offer a modal similar to the one found in the insight page so users can both add existing insights or create new insights to their dashboards from their dashboards.
Closes #19710
Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Currently if you have many insights (so that there are 2 or more pages on the saved insights page) you will not be able to see insights from a page other than the last viewed saved insights page. For example if you add insights until the second page is visible, navigate to this page on the saved insights view and then open the dashboard modal, you'll only see the insights from this second page.
Ah yes that makes complete sense now that I thought about it. I think the idea around isolating the responsibilities of the saved insights and modal logics to fetch insights would probably be the ideal solution here so what I did was implement a very simple loader into the modal logic to fetch and maintain insights isolated from savedInsightsLogic.
- The insights need to be paginated or we need to adapt the flow for a case when the user has many insights.
Yes, pagination makes sense. Just implemented a very simple pagination control into the modal.
- The "add text card" dropdown is missing from the button (see inline comment)
Oh yes, sorry about that just added it back. I had originally thought it was a placeholder but I see now it is the actual text cards on the dashboards.
3. Not a total blocker, but would be nice to have solved: If there are many insights the list displays a scrollbar, but there is no gap between the scrollbar and the button. This doesn't look nice. I guess this issue is already present in the "add to dashboard modal".
Added padding to the sides of the list container to give it some room.
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.
Sorry @jithenms, I've been bogged down in some other tasks and haven't gotten to this yet. I will make some time for it this week.
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.