Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate NameSelector from class based to function based component #2639

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

AdiIsHappy
Copy link
Contributor

Which problem is this PR solving?

Converted NameSelector in #2610

Description of the changes

  • Refactored class components to functional components for NameSelector.tsx.
  • Used React hooks in the component.
  • Updated the tests to work with the new component.

How was this change tested?

  • Tests are modified.

Checklist

@AdiIsHappy
Copy link
Contributor Author

AdiIsHappy commented Jan 27, 2025

@yurishkuro I have migrated the component from class to function. But in the test, there are two tests. particularly

  • attempts to focus the filter input when the component updates
  • throws Error when attempting to clear when required

which I am not able to convert; currently both are commented, and I am trying to fix them. other tests that required migration have been migrated.

Copy link

codecov bot commented Jan 27, 2025

Codecov Report

Attention: Patch coverage is 85.29412% with 5 lines in your changes missing coverage. Please review.

Project coverage is 96.57%. Comparing base (48733c2) to head (6898824).

Files with missing lines Patch % Lines
...s/jaeger-ui/src/components/common/NameSelector.tsx 85.29% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2639      +/-   ##
==========================================
- Coverage   96.60%   96.57%   -0.04%     
==========================================
  Files         255      255              
  Lines        7728     7727       -1     
  Branches     1990     1939      -51     
==========================================
- Hits         7466     7462       -4     
- Misses        262      265       +3     

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

@yurishkuro
Copy link
Member

Do we have a way of testing this in the UI?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants