Allow users to click on a date on the chart and pull sessions for that date#511
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds clickable traffic-chart data points that open a paginated Sessions drawer; introduces a Drawer UI, wires chart/context-menu actions to open sessions, adjusts chart tooltip/point behavior and DOM event wiring, adds session i18n, and refactors several list UIs and icon helpers. Changes
Sequence DiagramsequenceDiagram
actor User
participant Chart as Traffic Chart
participant TrafficView as TrafficView
participant SessionsDrawer as SessionsDrawer
participant API as /api/analytics
User->>Chart: click data point
Chart->>TrafficView: onDataPointClick({ x, index })
TrafficView->>TrafficView: compute from/to & label (timezone, bucket)
TrafficView->>SessionsDrawer: open(from, to, label, filters)
Note over SessionsDrawer: isOpen = true
SessionsDrawer->>API: POST action=getSessions (from,to,filters,skip=0,take=30)
API-->>SessionsDrawer: sessions page
SessionsDrawer->>SessionsDrawer: render list
User->>SessionsDrawer: scroll -> sentinel visible
SessionsDrawer->>API: POST getSessions (skip += 30)
API-->>SessionsDrawer: next page
SessionsDrawer->>SessionsDrawer: append results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/app/pages/Project/tabs/Traffic/SessionsDrawer.tsx`:
- Around line 77-100: The loadSessions async callback can apply results from
stale requests after the date/range changes; modify loadSessions (and the
similar logic around lines 102-113) to cancel or invalidate prior requests
before applying state by using an AbortController passed into fetchSessionsPage
or a monotonically increasing requestId captured when the call starts and
checked before calling setSessions/setHasMore; ensure fetchSessionsPage supports
AbortSignal or that you ignore results whose requestId doesn’t match the latest,
and clean up controllers on unmount or when dependencies (projectId, from, to,
timezone, filters) change.
- Around line 22-42: fetchSessionsPage can throw during fetch/response.json and
loadMore exits early without resetting loadingRef/loadingMore, which blocks
further pagination; update loadMore (the function controlling pagination and
using loadingRef and setLoadingMore) to wrap the await call to fetchSessionsPage
in try/catch/finally (or ensure a finally-equivalent cleanup) so that
loadingRef.current is set back to false and setLoadingMore(false) is called
regardless of success or error, and apply the same error-safe cleanup pattern to
the other pagination call site noted (the block around lines 115-126) to always
reset loading flags even when fetch or JSON parsing fails.
- Around line 65-66: The default parameter filters = [] creates a new array each
render which makes loadSessions unstable and re-triggers the initial-load
useEffect when isOpen is true; remove the default empty array from the
SessionsDrawerProps parameter and instead create a stableFilters value inside
the component (e.g., const stableFilters = useMemo(() => filters ?? [],
[filters]) or memoize any transformed filters) and use stableFilters wherever
loadSessions and the isOpen effect use filters (references: SessionsDrawerProps,
filters param, loadSessions function, isOpen-driven initial-load effect).
In `@web/app/pages/Project/tabs/Traffic/TrafficView.tsx`:
- Around line 337-379: The date range for the sessions drawer is being computed
from a plain dayjs(d.x) (in handleDataPointClick) which anchors startOf/endOf to
the viewer's local timezone; update the logic to first convert the point to the
project/dashboard timezone (e.g., using dayjs.tz or equivalent with the shared
timezone variable) then call startOf(...)/endOf(...).toISOString() and format
the label using the same tz-aware dayjs instance before calling
setSessionsDrawer({ from, to, label }); apply the same tz-aware fix to the other
occurrence around the session drawer code (the second block referenced).
- Around line 344-375: The switch on timeBucket currently falls through for
'week' and uses the 'day' range; add an explicit case 'week' in the switch (near
the existing cases for 'minute','hour','day','month','year') that sets from =
date.startOf('week').toISOString(), to = date.endOf('week').toISOString(), and
computes an appropriate label (for example a range like
date.startOf('week').format('MMM D') + ' - ' + date.endOf('week').format('MMM D,
YYYY') or "Week of " + date.startOf('week').format('MMM D, YYYY')) so the drawer
range for weekly chart points matches the chart bucket boundaries.
In `@web/app/pages/Project/View/ViewProject.helpers.tsx`:
- Around line 910-912: Replace the hardcoded tooltip hint "Click to explore
sessions" with a localized string from the app's i18n/locale mechanism (the same
key used for the menu label) inside the tooltip HTML template construction where
onDataPointClick is used; update both occurrences (the current template near the
join(...) block and the other instance noted around line 1007) to call the
localization function (e.g., t('...') or getString('...')) and inject its value
into the template instead of the literal text so non-English dashboards render
the translated hint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9008202-0c27-47ea-8c65-6b571dbc5ac6
📒 Files selected for processing (8)
web/app/pages/Project/View/ViewProject.helpers.tsxweb/app/pages/Project/View/components/ChartContextMenu.tsxweb/app/pages/Project/tabs/Sessions/Sessions.tsxweb/app/pages/Project/tabs/Traffic/SessionsDrawer.tsxweb/app/pages/Project/tabs/Traffic/TrafficChart.tsxweb/app/pages/Project/tabs/Traffic/TrafficView.tsxweb/app/styles/ProjectViewStyle.cssweb/public/locales/en.json
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
web/app/pages/Project/tabs/Sessions/Sessions.tsx (2)
216-223: Redundantsessionin useMemo dependencies.The
sessionobject is included in the dependency array, but the memo only usessession.isLive. SincesessionStartTime(which depends onsession.sessionStart) is already a dependency, includingsessionis redundant and could cause unnecessary recalculations when other session properties change.♻️ Proposed fix
}, [ - session, + session.isLive, language, timeFormat, sessionDurationString, sessionStartTime, t, ])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/tabs/Sessions/Sessions.tsx` around lines 216 - 223, The useMemo dependency array currently includes the entire session object, which is redundant; update the dependency list for the useMemo (the hook computing session duration/display) to remove session and instead depend on session.isLive (and keep sessionStartTime, language, timeFormat, sessionDurationString, and t) so the memo only re-runs when the live flag or the actual inputs change.
86-139: Duplicate helper components — see comment inProfiles.tsx.
BrowserIconandOSIconare duplicated here. As noted in the Profiles.tsx review, consider extracting these to shared components.Additionally, the fallback styling differs slightly between files:
- Here (line 96):
text-gray-400(no dark variant)- In Profiles.tsx (line 76):
text-gray-400 dark:text-gray-500🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/tabs/Sessions/Sessions.tsx` around lines 86 - 139, Extract the duplicated BrowserIcon and OSIcon into a shared component (e.g., SharedIcons.tsx) and import them into Sessions.tsx and the other file instead of duplicating logic; keep the same props (BrowserIcon: browser, className; OSIcon: os, theme, className) and preserve behavior for null/missing mappings. While extracting, normalize the fallback className to include the dark variant (use 'size-3.5 text-gray-400 dark:text-gray-500' as the default) so both usages render consistently, and ensure the exported components still return the same <img> or <GlobeIcon> branches (refer to BrowserIcon and OSIcon for exact branches to preserve).web/app/pages/Project/tabs/Profiles/Profiles.tsx (1)
66-119: Consider extractingBrowserIconandOSIconinto shared utility components.These helper components are duplicated nearly identically in
Sessions.tsx(lines 86-139). Extracting them to a shared location (e.g.,~/ui/BrowserIcon.tsxand~/ui/OSIcon.tsx) would reduce duplication and ensure consistent behavior across the codebase.Note a minor inconsistency:
BrowserIconfallback in this file usesdark:text-gray-500(line 76) while the same component inSessions.tsxuses onlytext-gray-400without a dark variant. This could cause visual inconsistency.♻️ Suggested shared component location
Create
~/ui/icons/BrowserIcon.tsxand~/ui/icons/OSIcon.tsxwith a consistent API, then import from both files:// ~/ui/icons/BrowserIcon.tsx import { GlobeIcon } from '@phosphor-icons/react' import { BROWSER_LOGO_MAP } from '~/lib/constants' interface BrowserIconProps { browser: string | null className?: string } export const BrowserIcon = ({ browser, className }: BrowserIconProps) => { const fallbackClass = className || 'size-3.5 text-gray-400 dark:text-gray-500' if (!browser) { return <GlobeIcon className={fallbackClass} weight='duotone' /> } const logoUrl = BROWSER_LOGO_MAP[browser as keyof typeof BROWSER_LOGO_MAP] if (!logoUrl) { return <GlobeIcon className={fallbackClass} weight='duotone' /> } return <img src={logoUrl} className={className || 'size-3.5'} alt={browser} /> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/tabs/Profiles/Profiles.tsx` around lines 66 - 119, BrowserIcon and OSIcon are duplicated here and in Sessions.tsx and have a minor fallback-class inconsistency (dark:text-gray-500 present only in this file); extract both components into shared files (e.g., ~/ui/icons/BrowserIcon.tsx and ~/ui/icons/OSIcon.tsx), move the logic from the local BrowserIcon and OSIcon into those shared components, update both Profiles.tsx and Sessions.tsx to import the shared components, and standardize the fallback class (e.g., 'size-3.5 text-gray-400 dark:text-gray-500') so both usages render consistently.web/app/pages/Project/View/ViewProject.helpers.tsx (1)
1092-1157: Consider logging errors in the catch block for debugging.The DOM listener attachment logic is well-guarded against duplicate registration via
__clickAttached. However, the emptycatchblock at lines 1155-1157 silently swallows all errors, which could make debugging difficult if something unexpected fails.🔧 Suggested improvement
- } catch { - // ignore - } + } catch (err) { + // Log for debugging but don't break chart rendering + if (process.env.NODE_ENV !== 'production') { + console.warn('[Chart] onrendered hook error:', err) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/View/ViewProject.helpers.tsx` around lines 1092 - 1157, The empty catch block swallows errors when attaching DOM listeners for onDataPointClick; update the catch to log the caught error (including context) using the existing logging mechanism or console.error so failures in the chart listener setup are visible — locate the try/catch around the block that reads chartInstance.$.svg, eventRectsGroup and __clickAttached, and replace the silent catch with code that logs a descriptive message and the error object (e.g., referencing chartInstance or eventRectsGroup in the message) while preserving the current suppression behavior otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/app/pages/Project/tabs/FeatureFlags/FeatureFlagsView.tsx`:
- Around line 151-160: The Badge currently renders hardcoded 'true'/'false'
strings for profile.lastResult inside FeatureFlagsView's JSX (the Badge element
rendering the span with CheckIcon/XIcon); replace those literals with localized
strings from the i18n translator (e.g., use the component's translation function
like t('featureFlags.true') / t('featureFlags.false') or equivalent keys) so the
badge label reads t(...) instead of 'true'/'false', keeping the icons and
conditional logic based on profile.lastResult unchanged.
---
Nitpick comments:
In `@web/app/pages/Project/tabs/Profiles/Profiles.tsx`:
- Around line 66-119: BrowserIcon and OSIcon are duplicated here and in
Sessions.tsx and have a minor fallback-class inconsistency (dark:text-gray-500
present only in this file); extract both components into shared files (e.g.,
~/ui/icons/BrowserIcon.tsx and ~/ui/icons/OSIcon.tsx), move the logic from the
local BrowserIcon and OSIcon into those shared components, update both
Profiles.tsx and Sessions.tsx to import the shared components, and standardize
the fallback class (e.g., 'size-3.5 text-gray-400 dark:text-gray-500') so both
usages render consistently.
In `@web/app/pages/Project/tabs/Sessions/Sessions.tsx`:
- Around line 216-223: The useMemo dependency array currently includes the
entire session object, which is redundant; update the dependency list for the
useMemo (the hook computing session duration/display) to remove session and
instead depend on session.isLive (and keep sessionStartTime, language,
timeFormat, sessionDurationString, and t) so the memo only re-runs when the live
flag or the actual inputs change.
- Around line 86-139: Extract the duplicated BrowserIcon and OSIcon into a
shared component (e.g., SharedIcons.tsx) and import them into Sessions.tsx and
the other file instead of duplicating logic; keep the same props (BrowserIcon:
browser, className; OSIcon: os, theme, className) and preserve behavior for
null/missing mappings. While extracting, normalize the fallback className to
include the dark variant (use 'size-3.5 text-gray-400 dark:text-gray-500' as the
default) so both usages render consistently, and ensure the exported components
still return the same <img> or <GlobeIcon> branches (refer to BrowserIcon and
OSIcon for exact branches to preserve).
In `@web/app/pages/Project/View/ViewProject.helpers.tsx`:
- Around line 1092-1157: The empty catch block swallows errors when attaching
DOM listeners for onDataPointClick; update the catch to log the caught error
(including context) using the existing logging mechanism or console.error so
failures in the chart listener setup are visible — locate the try/catch around
the block that reads chartInstance.$.svg, eventRectsGroup and __clickAttached,
and replace the silent catch with code that logs a descriptive message and the
error object (e.g., referencing chartInstance or eventRectsGroup in the message)
while preserving the current suppression behavior otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30c3941c-1ff1-4b9e-b9c1-63c98d64c6dd
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
web/app/pages/Project/View/ViewProject.helpers.tsxweb/app/pages/Project/tabs/Errors/ErrorsView.tsxweb/app/pages/Project/tabs/Experiments/ExperimentsView.tsxweb/app/pages/Project/tabs/FeatureFlags/FeatureFlagsView.tsxweb/app/pages/Project/tabs/Profiles/Profiles.tsxweb/app/pages/Project/tabs/Sessions/Sessions.tsxweb/app/pages/Project/tabs/Traffic/SessionsDrawer.tsxweb/app/pages/Project/tabs/Traffic/TrafficChart.tsxweb/app/pages/Project/tabs/Traffic/TrafficView.tsxweb/app/styles/ProjectViewStyle.cssweb/app/ui/Drawer.tsxweb/package.json
✅ Files skipped from review due to trivial changes (2)
- web/app/styles/ProjectViewStyle.css
- web/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- web/app/pages/Project/tabs/Traffic/SessionsDrawer.tsx
- web/app/pages/Project/tabs/Traffic/TrafficChart.tsx
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/community/src/analytics/analytics.service.ts`:
- Around line 179-181: The current check in analytics.service (the regex +
dayjs(date).isValid() call) uses dayjs non-strict parsing and can accept invalid
dates; change it to use dayjs with the customParseFormat plugin in strict mode:
import and extend dayjs with customParseFormat, then replace
dayjs(date).isValid() with dayjs(date, 'YYYY-MM-DDTHH:mm:ss', true).isValid()
(or the exact format string you expect for your ISO-like input) so parsing is
strict; keep the existing regex guard but ensure the strict-format check uses
the same format pattern.
In `@web/app/pages/Project/View/ViewProject.helpers.tsx`:
- Around line 1095-1097: The one-time guard using
eventRectsGroup.__clickAttached causes the attached click handlers to close over
stale columns and onDataPointClick; remove or change the one-time-only
attachment so handlers are rebound when chart props change (or instead attach a
stable wrapper that reads the latest columns and onDataPointClick from a mutable
ref stored on eventRectsGroup). Specifically, update the logic around
eventRectsGroup.__clickAttached (and the similar block covering the handlers at
1137–1172) to either (a) always reattach listeners each render, ensuring you
remove previous listeners first, or (b) attach a single stable listener that
references up-to-date values by reading
eventRectsGroup.currentColumns/eventRectsGroup.currentOnDataPointClick (or a
React ref) so clicks resolve to the latest columns and onDataPointClick
callback.
- Around line 911-913: The tooltip HTML builder currently appends a hint row
whenever onDataPointClick is set, which can render the literal "undefined" if
dataPointClickLabel is missing; update the conditional that builds the `<li>`
(where it currently uses onDataPointClick ? `<li...>${dataPointClickLabel}</li>`
: '') to require both onDataPointClick and a non-empty dataPointClickLabel
(e.g., onDataPointClick && dataPointClickLabel) so the hint is only included
when the label exists; apply the same guard to the other occurrence that uses
dataPointClickLabel.
In `@web/app/routes/api.analytics.ts`:
- Around line 82-88: The current validation is a no-op: you construct const date
= new Date(dateStr) but always return dateStr; change the branch so that when
Number.isNaN(date.getTime()) is false you return the normalized ISO timestamp
(use date.toISOString() to preserve a canonical full ISO 8601 timestamp) and
when the date is invalid do not forward the original value—instead throw a
validation error (or return a 400/bad-request response) so malformed dateStr
values are rejected; update the code that uses dateStr accordingly to handle the
thrown error/400.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b8e106a2-3eae-431f-8301-2ce04ce59979
📒 Files selected for processing (8)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/analytics.controller.tsbackend/apps/community/src/analytics/analytics.service.tsweb/app/pages/Project/View/ViewProject.helpers.tsxweb/app/pages/Project/tabs/Traffic/SessionsDrawer.tsxweb/app/pages/Project/tabs/Traffic/TrafficView.tsxweb/app/routes/api.analytics.ts
✅ Files skipped from review due to trivial changes (1)
- web/app/pages/Project/tabs/Traffic/SessionsDrawer.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web/app/pages/Project/tabs/Traffic/TrafficView.tsx
Comment on lines
+911
to
+913
| ).join( | ||
| '', | ||
| )}${onDataPointClick ? `<li class='pt-1 mt-1 border-t border-gray-200 dark:border-slate-700/80 text-[10px] text-gray-400 dark:text-slate-500'>${dataPointClickLabel}</li>` : ''}</ul>` |
There was a problem hiding this comment.
Guard tooltip hint text to avoid rendering undefined.
On Line 913 and Line 1008, the hint row is shown whenever onDataPointClick is set, even if dataPointClickLabel is missing. That can surface literal undefined in UI.
💡 Suggested fix
- )}${onDataPointClick ? `<li class='pt-1 mt-1 border-t border-gray-200 dark:border-slate-700/80 text-[10px] text-gray-400 dark:text-slate-500'>${dataPointClickLabel}</li>` : ''}</ul>`
+ )}${onDataPointClick && dataPointClickLabel ? `<li class='pt-1 mt-1 border-t border-gray-200 dark:border-slate-700/80 text-[10px] text-gray-400 dark:text-slate-500'>${dataPointClickLabel}</li>` : ''}</ul>`- ${onDataPointClick ? `<li class='pt-1 mt-1 border-t border-gray-200 dark:border-slate-700/80 text-[10px] text-gray-400 dark:text-slate-500'>${dataPointClickLabel}</li>` : ''}
+ ${onDataPointClick && dataPointClickLabel ? `<li class='pt-1 mt-1 border-t border-gray-200 dark:border-slate-700/80 text-[10px] text-gray-400 dark:text-slate-500'>${dataPointClickLabel}</li>` : ''}Also applies to: 1008-1008
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/app/pages/Project/View/ViewProject.helpers.tsx` around lines 911 - 913,
The tooltip HTML builder currently appends a hint row whenever onDataPointClick
is set, which can render the literal "undefined" if dataPointClickLabel is
missing; update the conditional that builds the `<li>` (where it currently uses
onDataPointClick ? `<li...>${dataPointClickLabel}</li>` : '') to require both
onDataPointClick and a non-empty dataPointClickLabel (e.g., onDataPointClick &&
dataPointClickLabel) so the hint is only included when the label exists; apply
the same guard to the other occurrence that uses dataPointClickLabel.
Comment on lines
+1095
to
+1097
| if (!eventRectsGroup || eventRectsGroup.__clickAttached) return | ||
| eventRectsGroup.__clickAttached = true | ||
|
|
There was a problem hiding this comment.
One-time listener attachment causes stale click/date mappings after rerenders.
Line 1095–1097 attaches handlers only once via __clickAttached. Those handlers close over the initial columns and onDataPointClick. After chart data/time updates, clicks can resolve to stale dates (or old callback behavior).
🔧 Suggested fix (rebind handlers with latest closures)
- const eventRectsGroup = svg.querySelector('.bb-event-rects')
- if (!eventRectsGroup || eventRectsGroup.__clickAttached) return
- eventRectsGroup.__clickAttached = true
+ type EventRectsGroup = SVGGElement & {
+ __handlers?: {
+ mousemove: (e: MouseEvent) => void
+ mouseleave: () => void
+ click: (e: MouseEvent) => void
+ }
+ }
+ const eventRectsGroup = svg.querySelector(
+ '.bb-event-rects',
+ ) as EventRectsGroup | null
+ if (!eventRectsGroup) return
+
+ if (eventRectsGroup.__handlers) {
+ eventRectsGroup.removeEventListener(
+ 'mousemove',
+ eventRectsGroup.__handlers.mousemove,
+ )
+ eventRectsGroup.removeEventListener(
+ 'mouseleave',
+ eventRectsGroup.__handlers.mouseleave,
+ )
+ eventRectsGroup.removeEventListener(
+ 'click',
+ eventRectsGroup.__handlers.click,
+ )
+ }
- eventRectsGroup.addEventListener(
- 'mousemove',
- (e: MouseEvent) => {
+ const mousemove = (e: MouseEvent) => {
// existing logic
- },
- )
+ }
+ eventRectsGroup.addEventListener('mousemove', mousemove)
- eventRectsGroup.addEventListener('mouseleave', () => {
+ const mouseleave = () => {
// existing logic
- })
+ }
+ eventRectsGroup.addEventListener('mouseleave', mouseleave)
- eventRectsGroup.addEventListener('click', (e: MouseEvent) => {
+ const click = (e: MouseEvent) => {
// existing logic (uses latest `columns` / `onDataPointClick`)
- })
+ }
+ eventRectsGroup.addEventListener('click', click)
+
+ eventRectsGroup.__handlers = { mousemove, mouseleave, click }Also applies to: 1137-1172
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/app/pages/Project/View/ViewProject.helpers.tsx` around lines 1095 - 1097,
The one-time guard using eventRectsGroup.__clickAttached causes the attached
click handlers to close over stale columns and onDataPointClick; remove or
change the one-time-only attachment so handlers are rebound when chart props
change (or instead attach a stable wrapper that reads the latest columns and
onDataPointClick from a mutable ref stored on eventRectsGroup). Specifically,
update the logic around eventRectsGroup.__clickAttached (and the similar block
covering the handlers at 1137–1172) to either (a) always reattach listeners each
render, ensuring you remove previous listeners first, or (b) attach a single
stable listener that references up-to-date values by reading
eventRectsGroup.currentColumns/eventRectsGroup.currentOnDataPointClick (or a
React ref) so clicks resolve to the latest columns and onDataPointClick
callback.
6 tasks
This was referenced May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
If applicable, please describe what changes were made in this pull request.
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit
New Features
UI Improvements
Localization
Bug Fixes