fix: ButtonBase for flex layouts #870
Draft
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
|
Cursor Agent can help with this pull request. Just |
Contributor
📖 Storybook Preview |
Contributor
📖 Storybook Preview |
- Add separate sections for horizontal (flex row) and vertical (flex column) layouts - Clarify that isFullWidth is for vertical layouts only - Document flex-1 as the correct approach for horizontal layouts - Add warning about semantic conflicts when using isFullWidth in rows - Update isFullWidth prop documentation with explicit guidance
georgewrmarshall
force-pushed
the
cursor/buttonbase-flex-layout-ad42
branch
from
c3b133c to
e3e19a4
Compare
Contributor
📖 Storybook Preview |
georgewrmarshall
changed the title
- Remove unused BoxBackgroundColor import - Remove unused Text import - Update FlexLayout story simplified to not need these imports
Contributor
📖 Storybook Preview |
- Add tests for function-based twClassName prop - Add tests for textClassName in normal and loading states - Add tests for iconClassName in loading state - Add tests for custom accessories in loading state - Fix ButtonIcon tests to work with new AnimatedPressable style structure 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Flatten style arrays in ButtonAnimated to avoid nested structure - Change from [[styles], animatedStyle] to [styles, animatedStyle] - Update ButtonIcon tests to use style[0] instead of style[0][0] - Simplifies style structure and makes tests more readable All tests pass with 100% coverage maintained. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
📖 Storybook Preview |
- Remove opacity-100 from spinner container expectations (not applied to container) - Remove content-container checks (wrapper was removed in refactoring) - Tests now correctly verify spinner presence and button disabled state All tests pass with 100% coverage maintained. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
📖 Storybook Preview |
Comment on lines
-2
to
+4
| import { View } from 'react-native'; | ||
|
|
||
| import { ButtonBaseSize } from '../../types'; | ||
| import { BoxFlexDirection, ButtonBaseSize } from '../../types'; | ||
| import { Box } from '../Box'; |
Contributor
Author
There was a problem hiding this comment.
Changing View to Box per conventions
Comment on lines
+137
to
+142
| export const FlexLayout: Story = { | ||
| render: () => ( | ||
| <Box flexDirection={BoxFlexDirection.Row} gap={4}> | ||
| <ButtonBase twClassName="flex-1">Lorem ipsum</ButtonBase> | ||
| <ButtonBase twClassName="flex-1">Lorem ipsum</ButtonBase> | ||
| </Box> |
Contributor
Author
There was a problem hiding this comment.
Adding flex layout story to check flex layout
- Add checks that text content has opacity-0 when loading - Restores the "hides content" part of "shows spinner + hides content" tests - Uses getByText and flattenStyles to verify opacity: 0 is applied All tests pass with 100% coverage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
📖 Storybook Preview |
Comment on lines
+73
to
+91
| it('applies function-based twClassName correctly', () => { | ||
| const twClassNameFn = (pressed: boolean) => | ||
| pressed ? 'bg-pressed' : 'bg-default'; | ||
|
|
||
| const tree = ReactTestRenderer.create( | ||
| <ButtonBase twClassName={twClassNameFn}>Function ClassName</ButtonBase>, | ||
| ); | ||
|
|
||
| const buttonAnimated = tree.root.findByProps({ | ||
| accessibilityRole: 'button', | ||
| }); | ||
| const styleFn = buttonAnimated.props.style as (p: { | ||
| pressed: boolean; | ||
| }) => unknown[]; | ||
|
|
||
| // Test both pressed states to cover the branch | ||
| expect(styleFn({ pressed: false })).toBeDefined(); | ||
| expect(styleFn({ pressed: true })).toBeDefined(); | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
Adding test coverage for the ternary operator on line 107 (typeof twClassName === 'function' ? twClassName(pressed) : twClassName) was never exercised in tests.
- Verify custom accessories have opacity-0 when isLoading=true - Use ReactTestRenderer to properly access wrapper props - Test now accurately verifies "in loading state" behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Verify textClassName and iconClassName functions work with pressed states - Test both pressed=false and pressed=true cases - Change from simple existence checks to actual behavior verification - Tests now accurately verify "applies" behavior they describe All tests pass with 100% coverage maintained. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
📖 Storybook Preview |
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.
Description
Refactors ButtonBase and ButtonAnimated to properly support flex layouts in React Native.
What is the reason for the change?
ButtonBase components couldn't use
flex-1in flex row layouts due to nested wrapper architecture in ButtonAnimated. Thew-autowidth constraint also prevented proper flex behavior.What is the improvement/solution?
Animated.createAnimatedComponent(Pressable)(68 lines vs 111)tw.style()for better readability and removed content wrapper Vieww-autotoself-startfor proper flex supportRelated issues
Fixes: (internal issue)
Manual testing steps
yarn storybook:iosoryarn storybook:androidflex-1Screenshots/Recordings
Before
Buttons couldn't use
flex-1in horizontal layouts - would either overflow or not stretch properly.After
Buttons properly support flex layouts with
twClassName="flex-1"in horizontal rows andisFullWidthin vertical columns.Screen.Recording.2026-01-15.at.10.00.55.AM.mov
Pre-merge author checklist
Pre-merge reviewer checklist