Skip to content

ROX-33443: Fix interactive legend click in Victory v37#19278

Draft
sachaudh wants to merge 4 commits intodv/ROX-28622-pf-6from
sc/ROX-33443-fix-interactive-legend-click
Draft

ROX-33443: Fix interactive legend click in Victory v37#19278
sachaudh wants to merge 4 commits intodv/ROX-28622-pf-6from
sc/ROX-33443-fix-interactive-legend-click

Conversation

@sachaudh
Copy link
Contributor

@sachaudh sachaudh commented Mar 3, 2026

Description

Jira: ROX-33443

getInteractiveLegendEvents' onLegendClick callback is never invoked when ChartBar is wrapped in ChartStack/ChartGroup alongside ChartAxis. This is an upstream PatternFly/Victory v37 bug: patternfly-react#12263.

Changes

  • Apply click events directly to ChartLegend via its events prop, bypassing the broken Chart-level event propagation
  • Fix chartNames ordering to match legend item indices (was using Object.values() which followed object declaration order instead of the array order used by the legend)
  • Add name prop to ChartBar components so the interactive legend can target chart children for hover effects
  • Unskip the component test for severity toggling and remove stale version pin comment

Root cause

Reproduced and narrowed down in sachaudh/pf-legend-click-repro:

Setup onLegendClick fires?
ChartArea + ChartStack + ChartAxis Yes
ChartBar + ChartAxis (no wrapper) Yes
ChartBar + ChartStack (no ChartAxis) Yes
ChartBar + ChartStack + ChartAxis No
ChartBar + ChartGroup + ChartAxis No

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

  • Verified severity toggling works by clicking legend items in the dashboard widget
  • Ran component test: all 3 specs pass, including the previously skipped toggling test

Screenshots

Screen.Recording.2026-03-05.at.11.24.47.AM.mov

@openshift-ci
Copy link

openshift-ci bot commented Mar 3, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@rhacs-bot
Copy link
Contributor

rhacs-bot commented Mar 3, 2026

Images are ready for the commit at c515037.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-240-gc515037a7c.

sachaudh added 2 commits March 5, 2026 11:55
Victory v37 (PF charts v8) broke getInteractiveLegendEvents -- the
onLegendClick callback passed through Chart's events prop is never
invoked. Work around this by attaching click events directly to the
ChartLegend component instead.

Also fixes chartNames ordering to match legend item indices and adds
name props to ChartBar for correct hover muting behavior.

Partially generated by AI.

Signed-off-by: Saif Chaudhry <schaudhr@redhat.com>
Update comment on the ChartLegend click workaround to reference
the upstream PatternFly issue (patternfly-react#12263) and describe
the precise trigger: ChartBar + ChartStack/ChartGroup + ChartAxis.

Partially generated by AI.

Signed-off-by: Saif Chaudhry <schaudhr@redhat.com>
@sachaudh sachaudh force-pushed the sc/ROX-33443-fix-interactive-legend-click branch from d5c4b8b to b54c022 Compare March 5, 2026 19:55
sachaudh added 2 commits March 5, 2026 12:05
The legend click workaround applied in the previous commit restores
severity toggling, so the skipped component test can be re-enabled.

Signed-off-by: Saif Chaudhry <schaudhr@redhat.com>
Explain that PF declares chartNames as a single-element tuple but
iterates it as a plain array at runtime, making the cast unavoidable.

Signed-off-by: Saif Chaudhry <schaudhr@redhat.com>
@codecov
Copy link

codecov bot commented Mar 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (dv/ROX-28622-pf-6@dac48f7). Learn more about missing BASE report.

Additional details and impacted files
@@                 Coverage Diff                  @@
##             dv/ROX-28622-pf-6   #19278   +/-   ##
====================================================
  Coverage                     ?   49.55%           
====================================================
  Files                        ?     2675           
  Lines                        ?   201838           
  Branches                     ?        0           
====================================================
  Hits                         ?   100015           
  Misses                       ?    94361           
  Partials                     ?     7462           
Flag Coverage Δ
go-unit-tests 49.55% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants