Refactor HelpMenu/hooks.ts for improved maintainability #107
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
RoyEJohnson
force-pushed
the
core-1450-refactor-helpmenu-hooks
branch
from
9c2f843 to
f4d7a81
Compare
RoyEJohnson
force-pushed
the
core-1450-refactor-helpmenu-hooks
branch
from
f4d7a81 to
7417805
Compare
RoyEJohnson
force-pushed
the
core-1450-refactor-helpmenu-hooks
branch
from
375ae3a to
3ee387a
Compare
jivey
approved these changes
Dec 23, 2025
Member
jivey
left a comment
jivey
left a comment
There was a problem hiding this comment.
This looks alright, maybe clean up some of the extraneous comments before merging.
| * Cleans up event listeners when closed. | ||
| */ | ||
| const checkClosed = setInterval(() => { | ||
| if ((popup.current as Window).closed) { |
Member
There was a problem hiding this comment.
This could be simplified to popup.current?.closed
| popup.current = null; | ||
| clearInterval(checkClosed); | ||
| } | ||
| }, 500); // Check every 500ms |
Member
There was a problem hiding this comment.
Claude loves to add superfluous comments like this that don't add a lot of value, I usually prefer to strip them out (or add instructions to only add high value comments.)
Splits the monolithic hooks.ts file into three focused modules following SOLID principles, particularly Single Responsibility Principle. **New Module Structure:** 1. **fieldMapping.ts** - Handles all Salesforce field mapping logic - Maps application fields to Salesforce hidden/visible fields - Parses user names into first/last name components - Determines field editability based on data source - Extensively commented to explain mapping decisions 2. **businessHours.ts** - Manages business hours tracking and formatting - Tracks current business hours with automatic timeout management - Formats hours for display using Intl.DateTimeFormat - Handles grace periods for better UX - Comments explain timeout logic and state management 3. **chatController.ts** - Controls chat popup window and messaging - Opens/positions popup window at bottom-right - Manages postMessage communication with security checks - Polls for popup closure and handles cleanup - Comments explain security considerations and lifecycle **Benefits:** - Each module has a single, clear responsibility - Easier to test individual components - Better code organization and navigation - Comprehensive comments explain WHY decisions were made - hooks.ts now serves as a clean re-export layer for backward compatibility Fixes CORE-1450 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Added tests for two uncovered scenarios in chatController.ts: - Line 154: Handles popup blocking when window.open returns null - Line 164: Ignores postMessage events from sources other than the opened popup window These tests improve the security and robustness of the chat controller by ensuring: 1. The controller gracefully handles popup blockers without throwing errors 2. PostMessage communication is validated to only accept messages from the intended popup window 🤖 Generated with [Claude Code](https://claude.com/claude-code) Add test coverage for chatController and businessHours branches - Add test for line 175 else branch in chatController.ts (when popup is NOT closed) - Add test for when nextState is undefined (not in business hours) - Add test for state reuse when hours haven't changed (object reference stability) These tests address the coverage gaps identified in code review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Update hooks.spec.tsx Add comprehensive test coverage for HelpMenu/index.tsx Added tests to cover previously untested code paths: - contactFormUrl memoization with URL encoding - showIframe state management and iframe rendering - PutAway button click handler - Message event listener registration and cleanup - Special character encoding in contact form parameters - NewTabIcon component export All tests pass and provide complete coverage for the component. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Add test coverage for businessHours line 90 else branch Added test case to cover the scenario where business hours change, ensuring the hook returns a new object reference when hours are different (testing the else branch that returns nextState). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Added tests to achieve 100% coverage of index.tsx: - Test for NewTabIcon SVG rendering with correct attributes - Test for CONTACT_FORM_SUBMITTED message event closing iframe - Test for custom children rendering in help menu - Test for chatConfig memoization behavior - Test for undefined chatConfig graceful handling - Test for openChat callback invocation when Chat With Us is clicked All tests pass with 100% statement, branch, function, and line coverage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
RoyEJohnson
force-pushed
the
core-1450-refactor-helpmenu-hooks
branch
from
fba46d1 to
b3cba8f
Compare
Removed inline comments that simply describe what the code does, keeping only comments that explain WHY decisions were made or provide important context about business logic, synchronization requirements, and edge cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
RoyEJohnson
force-pushed
the
core-1450-refactor-helpmenu-hooks
branch
from
290fdf7 to
d49c0bb
Compare
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
Refactors the monolithic
HelpMenu/hooks.tsfile into three focused modules following SOLID principles, particularly Single Responsibility Principle.Changes
New Module Structure
1. fieldMapping.ts - Salesforce Field Mapping
2. businessHours.ts - Business Hours Management
3. chatController.ts - Chat Popup Control
Backward Compatibility
The main
hooks.tsfile now serves as a clean re-export layer, maintaining full backward compatibility. All existing imports will continue to work without changes.Benefits
Code Quality
calculateBottomRightPosition)Testing
CORE-1450
🤖 Generated with Claude Code