feat: add show more buttons to page reports tiles #31143
Conversation
📸 UI snapshots have been updated3 snapshot changes in total. 0 added, 3 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
|
Size Change: -779 B (-0.01%) Total Size: 13.6 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR extracts modal handling logic into a dedicated component to support both web analytics and page reports tiles, enabling "Show more" buttons across different tile types.
- Created new
webAnalyticsModalLogic.tsthat serves as a bridge betweenwebAnalyticsLogicandpageReportsLogic - Added new
insightsUtils.tswithgetDashboardItemIdandgetNewInsightUrlFactoryfunctions - Modified
WebAnalyticsModal.tsxto use the new modal logic instead of directly usingwebAnalyticsLogic - Added
canOpenModal: trueproperty to query tiles inpageReportsLogic.tsto enable modal functionality - Updated
WebAnalyticsDashboard.tsxto use the new modal logic for consistent behavior across tile types
6 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
lricoy
force-pushed
the
feat/page-reports-add-show-more-modals
branch
from
2de679c to
de228f3
Compare
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
lricoy
requested review from
a team and
jabahamondes
and removed request for
a team
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
| const { modal } = useValues(webAnalyticsModalLogic) | ||
|
|
||
| const { setDates } = useActions(webAnalyticsLogic) | ||
| const { closeModal } = useActions(webAnalyticsModalLogic) |
There was a problem hiding this comment.
To guarantee you're connecting to the right logic - and the inner logic is connecting to the right webAnalyticsLogic - you should add a BindLogic to WebAnalyticsDashboard. This guarantees all child components connect to the same instance of the logic.
This isn't a requirement because the logic is unique now (their path + props tuple is constant) but it might be a good thing to do this now to avoid bugs in the future
|
Thanks @rafaeelaudibert! I just checked how the BindLogic works; I will add it later on to be safe about it |
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Important
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Problem
I've hidden those before because the modal-handling code we have was dedicated to the
webAnalyticsLogictiles, so it was not working outside of it givenPageReportsuse aSectionTileseparation instead of theTabTile.Changes
Does this work well for both Cloud and self-hosted?
Yes
How did you test this code?
Manually