Phase 2.4: Dropdown System Migration#2837
Merged
RoyEJohnson merged 11 commits intomainfrom Apr 10, 2026
Merged
Conversation
RoyEJohnson
force-pushed
the
phase-2-4-dropdown-migration
branch
from
8a1a166 to
cb582cc
Compare
RoyEJohnson
force-pushed
the
phase-2-4-dropdown-migration
branch
from
cb582cc to
2dfaa03
Compare
RoyEJohnson
force-pushed
the
phase-2-4-dropdown-migration
branch
from
2dfaa03 to
355c045
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the Dropdown component from styled-components to global CSS files, continuing Phase 2 of the styled-components migration initiative. The migration maintains full backward compatibility through styled() wrapper components while moving CSS to a separate, globally-imported CSS file for better maintainability and runtime performance.
Changes:
- Refactored Dropdown component to use plain React with CSS classes instead of styled-components
- Created new
Dropdown.cssfile with all dropdown styles, including complex focus-within selectors and fade-in animations - Added
Dropdown.cssimport tosrc/app/index.tsxin proper import order - Replaced styled-components with
classNamesfor className composition and wrapped exported components withstyled()for backward compatibility
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/app/components/Dropdown.tsx | Refactored component to use plain React with CSS classes; maintains backward compatibility with styled() wrappers for component selectors |
| src/app/components/Dropdown.css | New global CSS file containing all dropdown styling (animations, positioning, focus states, menu appearance) |
| src/app/index.tsx | Added Dropdown.css import in consistent order with other component CSS imports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
OpenStaxClaude
added a commit
that referenced
this pull request
Mar 31, 2026
This commit addresses all review comments from PR #2837: 1. Removed unused CSS classes (.dropdown-visually-hidden and .dropdown-visually-shown) - These utility classes were defined but never used anywhere in the component 2. Fixed critical bug in handleFocusOut - Changed condition from `if (e.relatedTarget && ...)` to `if (!e.relatedTarget || ...)` - This ensures dropdown menu closes when focus leaves to browser UI or another app - Previously, null relatedTarget would prevent setIsFocusWithin(false) from being called 3. Added test coverage for transient prop filtering - New test verifies that props starting with $ are filtered and not passed to DOM - Tests the else branch at line 31 of Dropdown.tsx 4. Added comprehensive test coverage for TabTransparentDropdown focus handlers - Test for handleFocusIn behavior - Test for handleFocusOut when focus moves outside (with relatedTarget) - Test for handleFocusOut when relatedTarget is null (critical bug fix scenario) - Test for maintaining focus when moving within the dropdown All tests verify the expected behavior and edge cases of the focus management system. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
RoyEJohnson
pushed a commit
that referenced
this pull request
Mar 31, 2026
This commit addresses all review comments from PR #2837: 1. Removed unused CSS classes (.dropdown-visually-hidden and .dropdown-visually-shown) - These utility classes were defined but never used anywhere in the component 2. Fixed critical bug in handleFocusOut - Changed condition from `if (e.relatedTarget && ...)` to `if (!e.relatedTarget || ...)` - This ensures dropdown menu closes when focus leaves to browser UI or another app - Previously, null relatedTarget would prevent setIsFocusWithin(false) from being called 3. Added test coverage for transient prop filtering - New test verifies that props starting with $ are filtered and not passed to DOM - Tests the else branch at line 31 of Dropdown.tsx 4. Added comprehensive test coverage for TabTransparentDropdown focus handlers - Test for handleFocusIn behavior - Test for handleFocusOut when focus moves outside (with relatedTarget) - Test for handleFocusOut when relatedTarget is null (critical bug fix scenario) - Test for maintaining focus when moving within the dropdown All tests verify the expected behavior and edge cases of the focus management system. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Update Dropdown.spec.tsx Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
RoyEJohnson
force-pushed
the
phase-2-4-dropdown-migration
branch
from
3c1f3f4 to
1c00da9
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
RoyEJohnson
pushed a commit
that referenced
this pull request
Mar 31, 2026
This commit addresses all review comments from PR #2837: 1. Removed unused CSS classes (.dropdown-visually-hidden and .dropdown-visually-shown) - These utility classes were defined but never used anywhere in the component 2. Fixed critical bug in handleFocusOut - Changed condition from `if (e.relatedTarget && ...)` to `if (!e.relatedTarget || ...)` - This ensures dropdown menu closes when focus leaves to browser UI or another app - Previously, null relatedTarget would prevent setIsFocusWithin(false) from being called 3. Added test coverage for transient prop filtering - New test verifies that props starting with $ are filtered and not passed to DOM - Tests the else branch at line 31 of Dropdown.tsx 4. Added comprehensive test coverage for TabTransparentDropdown focus handlers - Test for handleFocusIn behavior - Test for handleFocusOut when focus moves outside (with relatedTarget) - Test for handleFocusOut when relatedTarget is null (critical bug fix scenario) - Test for maintaining focus when moving within the dropdown All tests verify the expected behavior and edge cases of the focus management system. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Update Dropdown.spec.tsx Remove unnecessary transient prop filtering from DropdownToggleBase Analysis shows the transient prop filtering logic was unnecessary: 1. No code in the codebase passes transient props (starting with $) to DropdownToggle 2. All usages pass standard props like onClick, isOpen, tabIndex, className, component 3. The styled() wrapper would handle transient props automatically if they were used 4. The filtering code was dead code that could never execute its else branch Changes: - Simplified DropdownToggleBase to directly spread props without filtering - Removed test for transient prop filtering since it tested dead code - This addresses Roy's review comment about code coverage showing the else-branch not executing The component maintains the same functionality but with cleaner, simpler code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
RoyEJohnson
force-pushed
the
phase-2-4-dropdown-migration
branch
from
741db64 to
a57f43a
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
Review 17 Addressed: Comprehensive Dropdown Usage AuditI've completed a thorough audit of all code in the application that uses Dropdown (directly or indirectly) to ensure nothing was missed. Files That Import/Use Dropdown ComponentsI searched the entire codebase and found 20 files that import or use Dropdown-related components: ✅ Already Modified in This PR (8 files)
✅ No Changes Required (5 files)These files use Dropdown but do not require any changes because:
Files verified as compatible:
Test/Spec Files (7 files)These are test files that don't need changes: Non-Code Files (1 file)
Why No Additional Changes Are NeededThe migration was designed with full backward compatibility in mind:
VerificationI verified each file by:
ConclusionAll 20 files that use Dropdown have been accounted for:
The migration is comprehensive and complete. Every usage of Dropdown in the application has been reviewed and either updated or verified as compatible. Files checked: 20 total |
Contributor
Author
✅ All 17 Reviews Successfully AddressedReview 17 audit complete - verified all 20 files that use Dropdown. 8 files modified, 12 files confirmed compatible (no changes needed), 0 files missed. Full backward compatibility maintained via styled() wrappers. Migration is complete and ready for approval! |
jivey
approved these changes
Apr 2, 2026
Migrates dropdown components from styled-components to global CSS files. Changes: - Created Dropdown.css with all dropdown styles - Refactored Dropdown.tsx to use plain React with CSS classes - Updated index.tsx to import Dropdown.css globally - Maintained backward compatibility with styled() wrappers Components migrated: - DropdownToggle - Cursor styling - DropdownMenu - Positioning, shadow, border styles - DropdownFocusWrapper - Overflow control - DropdownList - Menu item styles and focus states - DotMenu (already partially migrated, verified) Key features preserved: - fadeIn animation (100ms ease-out) - Complex :focus-within selectors for tab-transparent variant - Component selector compatibility via styled() wrappers - Keyboard navigation and tab trapping - Controlled/uncontrolled state handling Follows migration patterns from PLAIN_CSS_MIGRATION_LEARNINGS.md: - Uses classnames package for className composition - React.forwardRef for components with refs - CSS variables set before ...style spread - Global CSS imported from src/app/index.tsx Related to: CORE-1698 🤖 Generated with [Claude Code](https://claude.com/claude-code) Delete package-lock.json Lint stuff Update yarn.lock Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit addresses all review comments from PR #2837: 1. Removed unused CSS classes (.dropdown-visually-hidden and .dropdown-visually-shown) - These utility classes were defined but never used anywhere in the component 2. Fixed critical bug in handleFocusOut - Changed condition from `if (e.relatedTarget && ...)` to `if (!e.relatedTarget || ...)` - This ensures dropdown menu closes when focus leaves to browser UI or another app - Previously, null relatedTarget would prevent setIsFocusWithin(false) from being called 3. Added test coverage for transient prop filtering - New test verifies that props starting with $ are filtered and not passed to DOM - Tests the else branch at line 31 of Dropdown.tsx 4. Added comprehensive test coverage for TabTransparentDropdown focus handlers - Test for handleFocusIn behavior - Test for handleFocusOut when focus moves outside (with relatedTarget) - Test for handleFocusOut when relatedTarget is null (critical bug fix scenario) - Test for maintaining focus when moving within the dropdown All tests verify the expected behavior and edge cases of the focus management system. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Update Dropdown.spec.tsx Remove unnecessary transient prop filtering from DropdownToggleBase Analysis shows the transient prop filtering logic was unnecessary: 1. No code in the codebase passes transient props (starting with $) to DropdownToggle 2. All usages pass standard props like onClick, isOpen, tabIndex, className, component 3. The styled() wrapper would handle transient props automatically if they were used 4. The filtering code was dead code that could never execute its else branch Changes: - Simplified DropdownToggleBase to directly spread props without filtering - Removed test for transient prop filtering since it tested dead code - This addresses Roy's review comment about code coverage showing the else-branch not executing The component maintains the same functionality but with cleaner, simpler code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit addresses Review 12 by Roy Johnson regarding TextResizerMenu styling. **Problem:** TextResizerMenu (a styled component) had a `&&` section trying to apply styles (background, right, left, top) that should affect its container (.dropdown-menu), but styled-components cannot style their container element. **Solution:** Added a `menuClassName` prop to the Dropdown component that allows consumers to pass a custom class name for the .dropdown-menu div. This enables proper styling customization without requiring migration of TextResizerMenu. **Changes:** 1. **src/app/components/Dropdown.tsx** - Added `menuClassName?: string` to Props interface - Updated TabHiddenDropDown to accept and apply menuClassName - Updated TabTransparentDropdown to accept and apply menuClassName - Used classNames() to merge 'dropdown-menu' with custom menuClassName 2. **src/app/components/Dropdown.css** - Added `.dropdown-menu.text-resizer-menu` styles - Applies background, right/left positioning, and top offset - These styles were previously in TextResizerMenu's `&&` section 3. **src/app/content/components/Topbar/TextResizer.tsx** - Added `menuClassName='text-resizer-menu'` prop to TextResizerDropdown - This applies the custom styles to the dropdown-menu div 4. **src/app/content/components/Topbar/styled.tsx** - Removed the `&&` section from TextResizerMenu - These styles are now applied to the parent .dropdown-menu via the class **Result:** - TextResizerMenu positioning now works correctly (right-aligned, no gap) - No migration of TextResizerMenu away from styled-components required - Clean separation: container styles in Dropdown.css, content styles in styled.tsx - Pattern established for other dropdowns that need custom menu positioning 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Applied the same menuClassName pattern used for TextResizerMenu to the ContextMenu situation. HighlightToggleEditContent was a styled wrapper component that applied styles (z-index, border, background-color, margin-bottom) to wrap the dropdown content. These styles belonged on the dropdown-menu container instead. 1. **Added .context-menu class in Dropdown.css** - z-index: 2 - border: 1px solid (formBorder) - background-color: (formBackground) - margin-bottom: 1rem (for last context menu spacing) 2. **Updated ContextMenu.tsx to use menuClassName** - Pass menuClassName='context-menu' to Dropdown component - Removed HighlightToggleEditContent wrapper component entirely - Styles now correctly apply to the .dropdown-menu container ✅ Clean separation of concerns (container styles on container) ✅ Removed unnecessary wrapper component ✅ Consistent pattern with TextResizerMenu solution ✅ Proper styling hierarchy This completes the menuClassName pattern for both special dropdown cases (TextResizer and ContextMenu). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Update Dropdown.css Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit addresses Review 14 which identified that the DotMenu dropdown positioning
needs the menuClassName treatment to properly position the dropdown-menu container.
### Problem
The DotMenu component had positioning CSS (`left`/`right`) applied to `.dot-menu-dropdown-list`
(the `<menu>` element), but this needs to be applied to the `.dropdown-menu` div container instead.
**Previous structure:**
```
<div class="dropdown-menu"> ← Needs positioning
<menu class="dot-menu-dropdown-list"> ← Had positioning (wrong level)
...
</menu>
</div>
```
### Solution
Applied the same `menuClassName` pattern used for TextResizerMenu and ContextMenu:
1. **Updated DotMenuDropdown component (DotMenu.tsx):**
- Detects `rightAlign` prop from DotMenuDropdownList children
- Passes appropriate `menuClassName` to Dropdown:
- `dot-menu-right-align` when rightAlign is true
- `dot-menu-left-align` when rightAlign is false
2. **Updated DotMenu.css:**
- Changed from `.dot-menu-dropdown .dot-menu-dropdown-list.dot-menu-right-align`
- To: `.dropdown-menu.dot-menu-right-align`
- Changed from `.dot-menu-dropdown .dot-menu-dropdown-list:not(.dot-menu-right-align)`
- To: `.dropdown-menu.dot-menu-left-align`
### Result
✅ Positioning now correctly applied to `.dropdown-menu` container
✅ Consistent with TextResizerMenu and ContextMenu patterns
✅ Proper separation: container positioning in Dropdown.css, content styles remain in DotMenu.css
✅ Backward compatible with existing rightAlign functionality
---
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Applied the menuClassName pattern to the DisplayNote dropdown to fix the left positioning issue.
The DisplayNote component had `left: -4rem` being applied to `DropdownList` via styled-components:
```css
${DropdownList}${DropdownList} {
left: -4rem;
}
```
However, this positioning needs to be applied to the `.dropdown-menu` container for proper positioning after the migration to global CSS.
Applied the same menuClassName pattern successfully used for TextResizerMenu, ContextMenu, and DotMenu:
1. **Created `.display-note-menu` class in Dropdown.css:**
- Added `left: -4rem` positioning to `.dropdown-menu.display-note-menu`
2. **Updated DisplayNote.tsx:**
- Added `menuClassName='display-note-menu'` prop to Dropdown component
- Removed the old styled-component CSS that targeted DropdownList
✅ **Correct positioning**: Dropdown menu now properly positions with left offset
✅ **Consistent pattern**: Same menuClassName approach as other custom dropdowns
✅ **Proper architecture**: Container positioning belongs on the container element
✅ **Clean code**: Removed unnecessary styled-component selector complexity
This completes the menuClassName pattern for all dropdown types requiring custom positioning (TextResizer, ContextMenu, DotMenu, and DisplayNote).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Update Dropdown.css
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
RoyEJohnson
force-pushed
the
phase-2-4-dropdown-migration
branch
from
e79d86b to
e1efe3f
Compare
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
16 tasks
OpenStaxClaude
added a commit
that referenced
this pull request
Apr 13, 2026
Moves position:relative from the styled-components wrapper to CSS file with appropriate class selectors. This prevents the Dropdown component from creating a containing block that interferes with sticky positioning of ancestor elements. The position:relative style is now applied via CSS classes: - .dropdown-hidden for TabHiddenDropDown - .dropdown-transparent for TabTransparentDropdown This maintains dropdown menu positioning functionality while allowing parent elements like TopBarWrapper and BookBanner BarWrapper to maintain their sticky positioning behavior on mobile. Fixes regression introduced in PR #2837. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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
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
Migrates dropdown components from styled-components to global CSS files, continuing Phase 2 of the styled-components migration initiative.
Related Jira Ticket: CORE-1698
Changes
New Files
Modified Files
Components Migrated
Key Features Preserved
✅ fadeIn animation (100ms ease-out) for dropdown appearance
✅ Complex :focus-within selectors for tab-transparent dropdown variant
✅ Component selector compatibility via styled() wrappers
✅ Keyboard navigation and tab trapping
✅ Controlled/uncontrolled state handling
✅ Safari focus handling (callOrRefocus function)
Migration Patterns Followed
Following patterns from
PLAIN_CSS_MIGRATION_LEARNINGS.md:✅ Uses
classnamespackage for className composition✅ Wrapped components with
styled()for backward compatibility✅ Used
React.forwardReffor components that receive refs✅ Filtered transient props (starting with $) before spreading to DOM
✅ Set CSS variables before
...stylespread✅ Imported global CSS from
src/app/index.tsxBackward Compatibility
Maintained full backward compatibility:
styled()- used in component selectorsstyled()- used in component selectors in highlights and filtersAll existing consumers will continue to work without modification.
Testing
Files Using Dropdown Components
Test Plan
Manual testers should verify:
Visual verification
Interactive behavior
Keyboard accessibility
Responsive behavior
Impact
🤖 Generated with Claude Code