refactor: improve overlay select and tabs#50
Conversation
Remove OverlayStateMixin and inline session tracking directly into NakedMenu and NakedSelect for simpler, self-contained state management. Add configurable pageJumpSize to NakedSelect and extract NakedTextField build method into smaller focused helpers.
|
To view this pull requests documentation preview, visit the following URL: Documentation is deployed and generated using docs.page. |
Rename _defaultPageJumpFallback to _maxPageJumpSize for clarity. Simplify textfield helper methods by accessing _effectiveController and _effectiveFocusNode directly instead of threading them as params.
There was a problem hiding this comment.
Pull request overview
This PR refactors overlay-based widgets (NakedSelect, NakedMenu, and NakedTabs) and NakedTextField to improve code organization and performance. The refactoring inlines the OverlayStateMixin directly into the widgets that use it, adds a configurable pageJumpSize parameter to NakedSelect for PageUp/PageDown navigation, and optimizes focus iteration in NakedTabs by using dynamic bounds based on actual traversal descendant counts. The NakedTextField refactoring extracts the large build method into smaller, more maintainable helper methods without changing functionality.
Changes:
- Refactored NakedTextField's
buildmethod into smaller helper methods for better code organization - Inlined
OverlayStateMixininto NakedSelect and NakedMenu states, removing the shared mixin - Added configurable
pageJumpSizeparameter to NakedSelect for customizable PageUp/PageDown navigation - Optimized focus iteration in NakedTabs by calculating dynamic iteration limits based on traversal descendants
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/naked_ui/lib/src/naked_textfield.dart | Refactored large build method into focused helper methods (_buildInputFormatters, _buildEditableText, _wrapEditable, _handleSemanticTap, _wrapSemantics, _buildContent, _wrapFocusSemantics, _wrapSelectionGestureDetector, _wrapMouseRegion) for improved maintainability |
| packages/naked_ui/lib/src/naked_tabs.dart | Changed from static iteration limit (100) to dynamic limit based on traversalDescendants.length for more accurate focus traversal bounds |
| packages/naked_ui/lib/src/naked_select.dart | Inlined OverlayStateMixin functionality, added pageJumpSize parameter with validation, implemented _pageJumpSizeForScope method to calculate dynamic jump sizes, and updated documentation |
| packages/naked_ui/lib/src/naked_menu.dart | Inlined OverlayStateMixin functionality, directly implementing selection tracking and focus management logic |
| packages/naked_ui/lib/src/base/overlay_base.dart | Removed OverlayStateMixin as it's now inlined into the widgets that used it |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The parity tests hardcoded SystemMouseCursors.click for Material enabled widgets. Flutter 3.41.2 changed the default cursor, breaking CI. Now the tests query Material's actual cursor and assert Naked matches it, which is the correct parity behavior.
LCOV of commit
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Number of items to jump for PageUp/PageDown. | ||
| /// | ||
| /// When null, the jump size is derived from the number of traversal | ||
| /// descendants in the overlay's focus scope, capped to a reasonable | ||
| /// upper bound. | ||
| final int? pageJumpSize; |
There was a problem hiding this comment.
The new pageJumpSize parameter lacks test coverage. While the default behavior (using traversalDescendants.length) is tested indirectly through existing PageUp/PageDown tests, there are no tests verifying that a custom pageJumpSize value is respected. Consider adding a test that sets an explicit pageJumpSize (e.g., 5) and verifies that PageUp/PageDown move focus by exactly that amount.
LCOV of commit
|
Description
Refactors overlay-based select and tabs widgets for improved code clarity and performance. Inlines the overlay state mixin and improves documentation for focus management behavior. Also optimizes focus iteration in tabs to avoid redundant method calls.
Related Issues
Improves code maintainability and performance of NakedSelect and NakedTabs widgets.
Checklist
///).Breaking Change