Refactor: Migrate ViewingLayer to Functional Component#3504
Refactor: Migrate ViewingLayer to Functional Component#3504M-DEV-1 wants to merge 10 commits intojaegertracing:mainfrom
Conversation
Signed-off-by: M-DEV-1 <mahadevankizhakkedathu@gmail.com>
There was a problem hiding this comment.
Pull request overview
This pull request migrates the ViewingLayer component from a React class-based component to a functional component using hooks, as part of the larger modernization effort to refactor Jaeger UI's codebase (#2610).
Changes:
- Converted class component to functional component using
forwardRef - Replaced
this.statewithuseStatefor thepreventCursorLinestate - Replaced
componentWillUnmountwithuseEffectfor cleanup logic - Added
useImperativeHandleto expose component methods for backwards compatibility with existing tests - Wrapped component in
React.memoto preservePureComponentperformance optimization - Used
useRefpattern withpropsRefto ensure handlers have access to latest props
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: mahadevan <135952571+M-DEV-1@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: M-DEV-1 <mahadevankizhakkedathu@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: M-DEV-1 <mahadevankizhakkedathu@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('prevents the cursor from being drawn on scrubber mouseleave', async () => { | ||
| fireEvent.mouseLeave(container.querySelectorAll('[data-testid="scrubber"]')[0]); | ||
| expect(ref.current.state.preventCursorLine).toBe(false); | ||
| await waitFor(() => | ||
| expect(container.querySelector('.ViewingLayer--cursorGuide')).toBeInTheDocument() | ||
| ); | ||
| }); |
There was a problem hiding this comment.
The test name "prevents the cursor from being drawn on scrubber mouseleave" no longer matches the asserted behavior (it waits for the cursor guide to be present after mouseleave). Renaming the test to reflect the actual expectation (e.g. cursor guide is restored on mouseleave) will reduce confusion when debugging failures.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3504 +/- ##
==========================================
- Coverage 88.32% 88.31% -0.01%
==========================================
Files 299 299
Lines 9487 9496 +9
Branches 2420 2420
==========================================
+ Hits 8379 8386 +7
- Misses 1105 1107 +2
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @yurishkuro, just bumping this PR in case it got buried. would really appreciate a review when you have time. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('prevents the cursor from being drawn on scrubber mouseleave', async () => { | ||
| fireEvent.mouseLeave(container.querySelectorAll('[data-testid="scrubber"]')[0]); | ||
| expect(ref.current.state.preventCursorLine).toBe(false); | ||
| await waitFor(() => | ||
| expect(container.querySelector('.ViewingLayer--cursorGuide')).toBeInTheDocument() | ||
| ); | ||
| }); |
There was a problem hiding this comment.
This test name no longer matches the assertion: on mouseLeave the cursor guide is expected to re-appear (i.e., cursor drawing is no longer prevented). Renaming the test to reflect the intended behavior will make failures easier to interpret.
|
Also, since I've been periodically updating the branch with |
Which problem is this PR solving?
Description of the changes
ViewingLayerfrom a React Class-Based Component to a Functional Component in adherence to the acceptance criterion as described in [UI Refactor] Migrate ViewingLayer to Functional Component #3371How was this change tested?
Unit Tests
Visuals
Videos
Screencast.from.2026-02-06.02-32-03.webm
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test