Skip to content

Conversation

@Parship999
Copy link

@Parship999 Parship999 commented May 25, 2025

Which problem is this PR solving?

While working on some other issues in the codebase, I found additional components with similar React Fragment and key issues. These components had the same React Fragment and key patterns (like in #2812 ) that needed to be solved for consistency and react best practices

Description of the changes

  1. KeyValuesTable.tsx
  • Changed <> to <React.Fragment key={i}> for proper key attribution
  1. highlightMatches.tsx
  • Ensured consistent return types by wrapping plain strings in <span> elements
  • Added proper keys to all elements returned from map function
  1. SearchResults/index.tsx
  • Added keys to React.Fragment elements in conditional rendering

Checklist

@Parship999 Parship999 requested a review from a team as a code owner May 25, 2025 17:14
@Parship999 Parship999 requested review from mahadzaryab1 and removed request for a team May 25, 2025 17:14
@codecov
Copy link

codecov bot commented May 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.39%. Comparing base (483d574) to head (5b97c8e).
Report is 14 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2823      +/-   ##
==========================================
+ Coverage   97.31%   97.39%   +0.07%     
==========================================
  Files         256      256              
  Lines        7947     7975      +28     
  Branches     2077     2062      -15     
==========================================
+ Hits         7734     7767      +33     
+ Misses        213      208       -5     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Parship Chowdhury <[email protected]>
@Parship999
Copy link
Author

I created this PR because I noticed several places in the codebase where React fragments were used without keys inside lists, which can cause warnings in React, especially as the codebase evolvess. Acc to me these changes are about following React best practices for keys and rendering

@yurishkuro
Copy link
Member

I don't have a problem with the overall direction, I asked specific question.

@Parship999
Copy link
Author

Thanks for clarifying, I understand your feedbacks and will make the changes as suggested.

Parship12 added 2 commits May 31, 2025 18:33
Signed-off-by: Parship Chowdhury <[email protected]>
Signed-off-by: Parship Chowdhury <[email protected]>
@Parship999 Parship999 requested a review from yurishkuro May 31, 2025 13:50
Signed-off-by: Parship Chowdhury <[email protected]>
@yurishkuro yurishkuro added the changelog:bugfix-or-minor-feature 🐞 Bug fixes, Minor Improvements label May 31, 2025
@yurishkuro yurishkuro enabled auto-merge May 31, 2025 16:26
@yurishkuro yurishkuro added this pull request to the merge queue May 31, 2025
Merged via the queue into jaegertracing:main with commit 0147179 May 31, 2025
11 of 12 checks passed
@Parship999 Parship999 deleted the fix-react-fragment-keys-comprehensive branch May 31, 2025 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:bugfix-or-minor-feature 🐞 Bug fixes, Minor Improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants