Skip to content

Extend the key fix from https://github.com/recharts/recharts/pull/6623 to the other pages that suffer from the same bug#6637

Merged
PavelVanecek merged 1 commit intomainfrom
www-editor-state-clear
Nov 16, 2025
Merged

Extend the key fix from https://github.com/recharts/recharts/pull/6623 to the other pages that suffer from the same bug#6637
PavelVanecek merged 1 commit intomainfrom
www-editor-state-clear

Conversation

@PavelVanecek
Copy link
Collaborator

@PavelVanecek PavelVanecek commented Nov 16, 2025

Description

API and Guide pages were also affected by the same bug.

Related Issue

#6618

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved page navigation to properly reset component state when switching between pages, ensuring a fresh experience without residual data from previous pages.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 16, 2025

Walkthrough

Two React view components (APIViewNew and GuideView) now include key props tied to the current page on their main containers to force component remounts during page navigation.

Changes

Cohort / File(s) Summary
Page-based key props for remounting
www/src/views/APIViewNew.tsx, www/src/views/GuideView.tsx
Added key={page} to main containers (content div and Guide component, respectively) to trigger remounts when page changes

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Verify that the page variable is consistently managed and reflects the current navigation state
  • Confirm that forcing remounts via key changes does not inadvertently reset necessary component state or break existing functionality
  • Ensure no performance degradation from repeated remounting behavior

Possibly related PRs

Suggested reviewers

  • ckifer

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete and missing several required template sections including motivation/context, testing details, type of change, and checklist items. Add missing sections: explain the bug and why the fix is needed, describe how changes were tested, select the change type (bug fix), and complete the checklist items.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: extending a previous key fix to additional pages affected by the same bug.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch www-editor-state-clear

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 19ea6fd and 95d9afa.

📒 Files selected for processing (2)
  • www/src/views/APIViewNew.tsx (1 hunks)
  • www/src/views/GuideView.tsx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build, Test, Pack
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
www/src/views/APIViewNew.tsx (1)

198-198: LGTM! Key prop forces remount on page navigation.

The addition of key={page} to the content div will cause React to unmount and remount the entire content tree whenever the page parameter changes, effectively resetting any internal component state. This is the correct pattern for addressing state-persistence bugs during navigation.

Please verify that:

  1. This pattern matches the fix applied in PR fix stuck chart issue #6623 (the referenced fix)
  2. The bug described in issue [Bug] page navigation is broken after first chart load #6618 is resolved with this change

You can test by navigating between different API pages and confirming that stale state no longer persists.

www/src/views/GuideView.tsx (1)

43-43: LGTM! Consistent with the APIViewNew fix.

Adding key={page} to the Guide component ensures it remounts when navigating between different guide pages, preventing state from the previous guide from persisting. This approach is consistent with the fix in APIViewNew.tsx, though applied at the component level rather than on a wrapper div—both are valid and achieve the same goal.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Nov 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.15%. Comparing base (19ea6fd) to head (95d9afa).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6637   +/-   ##
=======================================
  Coverage   94.15%   94.15%           
=======================================
  Files         493      493           
  Lines       41104    41104           
  Branches     4783     4782    -1     
=======================================
  Hits        38700    38700           
  Misses       2399     2399           
  Partials        5        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.

@PavelVanecek PavelVanecek merged commit f99630d into main Nov 16, 2025
27 of 28 checks passed
@PavelVanecek PavelVanecek deleted the www-editor-state-clear branch November 16, 2025 05:24
@codecov
Copy link

codecov bot commented Nov 16, 2025

Bundle Report

Bundle size has no change ✅

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.

1 participant