-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor Topbar - Convert to Modern Hooks #2636
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
Open
OpenStaxClaude
wants to merge
5
commits into
main
Choose a base branch
from
core-1456-refactor-topbar-phase1
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Convert Topbar component from legacy connect() HOC pattern to modern useSelector/useDispatch hooks pattern, aligning with 95% of components in the rex-web codebase. Changes: - Replace connect() HOC with useSelector for reading Redux state - Replace mapDispatchToProps with useDispatch for dispatching actions - Remove lodash/fp/flow composition in favor of direct dispatch calls - Remove Props interface (no longer needed with hooks) - Update CommonSearchInputParams type to not depend on Props - Add comprehensive JSDoc comment explaining component purpose Benefits: - Aligns with modern React patterns used throughout codebase - Eliminates prop drilling from HOC - Simpler, more direct code - Better tree-shaking and code splitting - Easier to understand and maintain No functional changes - maintains all existing behavior. Related to CORE-1452 and CORE-1456 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
15be302 to
2510ae2
Compare
2510ae2 to
91fe4e8
Compare
91fe4e8 to
668efaa
Compare
668efaa to
c984e21
Compare
c984e21 to
b7774b2
Compare
b7774b2 to
8e583ee
Compare
Extract business logic into reusable custom hooks following Single Responsibility Principle: Created hooks.ts with two custom hooks: - useSearchState: Manages search query state and synchronization with Redux - Handles complex sync between local state (for input responsiveness) and Redux state - Provides handlers for search change, clear, and submit - Eliminates manual synchronization logic from component - useMobileToolbar: Manages mobile toolbar open/close state - Encapsulates toggle logic and search clearing behavior - Simplifies mobile-specific state management Updated index.tsx to use new hooks: - Replaced inline state management with hook calls - Removed duplicate event handler logic - Cleaned up imports (removed unused action creators) - Component now focuses on composition rather than logic Benefits: - Better separation of concerns (logic vs presentation) - Each hook testable in isolation - Reduced component complexity (removed 40+ lines of logic) - Improved code reusability - Maintained SSR-safe conditional render of AltSCycler (typeof window check) No functional changes - all existing behavior preserved. Related to CORE-1456 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
The handleSearchClear function was incorrectly dispatching clearSearch() action, which resets all search state including closing the sidebar. The test 'clears search input without affecting search results sidebar' expects that clicking the clear button should: 1. Clear the search input text 2. Keep the search results sidebar open Fixed by removing dispatch(clearSearch()) and only clearing local state (query and formSubmitted). This matches the original behavior before the refactoring. Fixes failing test in SearchResultsSidebar/index.browserspec.tsx
8b8a56d to
85e2795
Compare
RoyEJohnson
approved these changes
Dec 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1 of Topbar refactoring: Convert from legacy
connect()HOC pattern to modern hooks pattern.This is the first phase of the refactoring proposed in CORE-1456. Roy requested each phase in a separate PR for easier review.
Changes Made
Converted the Topbar component from the legacy
react-reduxconnect()HOC pattern to modern hooks:connect()HOC withuseSelectoranduseDispatchPropsinterface - no longer needed with hookslodash/fp/flowcomposition - direct dispatch calls insteadCode Comparison
Before (connect HOC):
After (hooks):
Benefits
connect()Pattern Alignment
This change aligns Topbar with modern components in the codebase like:
BookBanner.tsx- uses hooksToolbar/index.tsx- uses hooksSidebarControl/index.tsx- uses hooksNext Phases (Future PRs)
Testing
Related Issues
🤖 Generated with Claude Code