generated from MetaMask/metamask-module-template
-
-
Notifications
You must be signed in to change notification settings - Fork 7
Open
Labels
Description
Description
Two issues discovered in ButtonBase component during ButtonHero development:
- Performance Issue:
textClassNameprop forces function creation even for static values, causing unnecessary re-renders - Loading Text Bug: When
isLoading=trueandloadingTextis provided, the button displays both the loading text AND the children text instead of replacing the children
Steps to Reproduce
Issue 1 - textClassName Performance:
- Create a ButtonBase component with static text styling
- Try to use
textClassName="text-primary-inverse" - Get TypeScript error:
Type 'string' is not assignable to type '(pressed: boolean) => string' - Must use
textClassName={() => 'text-primary-inverse'}creating new function on every render
Issue 2 - Loading Text Display:
- Create a ButtonBase with
children="Click Me" - Set
isLoading={true}andloadingText="Loading..." - Render the component
- Observe both texts are displayed: "Loading...Click Me"
Expected Behavior
Issue 1: textClassName should accept both string and function:
// Should work for static values
textClassName="text-primary-inverse"
// Should work for dynamic values
textClassName={(pressed) => pressed ? 'text-pressed' : 'text-default'}Issue 2: Loading text should replace children, not append to them:
- Expected: Button displays only "Loading..."
- Actual: Button displays "Loading...Click Me"
Screenshots
Test Output showing the bug:
● ButtonHero › handles loading state correctly
expect(element).toHaveTextContent()
Expected element to have text content:
loading...
Received:
loading...Test Button
Accessibility output during loading:
<View accessibilityHint="Button is currently loading, please wait"
accessibilityLabel="loading..." accessibilityRole="button" accessibilityState={
{ "busy": true, "disabled": true, } } accessible={true} testID="loading-button"
/>Environment
- OS: macOS 24.5.0
- React Native: MetaMask Mobile app
- Package:
@metamask/design-system-react-native - Component:
ButtonBase
Additional Context
Issue 1 Impact:
- Forces unnecessary function creation on every render
- Prevents memoization optimizations
- Makes API less ergonomic for simple use cases
- Common anti-pattern in React performance
Issue 2 Impact:
- Poor UX - confusing display during loading states
- Accessibility issues - screen readers get conflicting information
- May cause layout issues with longer text combinations
Suggested Fixes:
- textClassName API: Change type to
string | ((pressed: boolean) => string) - Loading behavior: When
isLoading=true, only show loading content, hide children
Workaround Currently Used:
// Issue 1 workaround
const getTextClassName = useCallback(() => 'text-primary-inverse', []);
textClassName = { getTextClassName };
// Issue 2 workaround
// Test for loading text presence rather than exact text content
expect(getByText('loading...')).toBeOnTheScreen();Related Files:
app/component-library/components-temp/Buttons/ButtonHero/ButtonHero.tsxapp/component-library/components-temp/Buttons/ButtonHero/ButtonHero.test.tsx
Both issues were discovered during implementation of a new ButtonHero component in MetaMask Mobile that extends ButtonBase functionality from @metamask/design-system-react-native.