-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix(TMCU-1045): Remove useElevatedSurface shim from Core UX bottom sheets #33162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
3fb3c08
f4ff3b8
a8b92b9
b473132
6a83c16
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -361,17 +361,9 @@ jest.mock('../../../util/navigation/navUtils', () => ({ | |
| useParams: () => mockUseParams(), | ||
| })); | ||
|
|
||
| let mockIsPureBlack = false; | ||
|
|
||
| jest.mock('@metamask/design-system-twrnc-preset', () => ({ | ||
| ...jest.requireActual('@metamask/design-system-twrnc-preset'), | ||
| usePureBlack: () => mockIsPureBlack, | ||
| })); | ||
|
|
||
| describe('TradeWalletActions', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| mockIsPureBlack = false; | ||
| mockParentCanGoBack = true; | ||
| jest | ||
| .spyOn(global, 'requestAnimationFrame') | ||
|
|
@@ -445,42 +437,6 @@ describe('TradeWalletActions', () => { | |
| ).toBeNull(); | ||
| }); | ||
|
|
||
| it('renders a bottom cutout stroke when pure black is enabled', () => { | ||
| mockIsPureBlack = true; | ||
|
|
||
| const { getByTestId } = renderScreen( | ||
| TradeWalletActions, | ||
| { | ||
| name: 'TradeWalletActions', | ||
| }, | ||
| { | ||
| state: mockInitialState, | ||
| }, | ||
| ); | ||
|
|
||
| expect( | ||
| getByTestId(WalletActionsBottomSheetSelectorsIDs.MENU_BOTTOM_STROKE), | ||
| ).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('does not render the bottom cutout stroke when pure black is disabled', () => { | ||
| mockIsPureBlack = false; | ||
|
|
||
| const { queryByTestId } = renderScreen( | ||
| TradeWalletActions, | ||
| { | ||
| name: 'TradeWalletActions', | ||
| }, | ||
| { | ||
| state: mockInitialState, | ||
| }, | ||
| ); | ||
|
|
||
| expect( | ||
| queryByTestId(WalletActionsBottomSheetSelectorsIDs.MENU_BOTTOM_STROKE), | ||
| ).toBeNull(); | ||
| }); | ||
|
|
||
| it('should render earn button if the stablecoin lending feature is enabled', () => { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Revert the changes to this file
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cursoragent Revert the changes to this file
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted
Pushed in commit Note: Those two tests will fail until the related changes are also reverted in:
|
||
| ( | ||
| selectStablecoinLendingEnabledFlag as jest.MockedFunction< | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,13 +36,8 @@ import { | |
| TextVariant, | ||
| } from '@metamask/design-system-react-native'; | ||
| import { | ||
| usePureBlack, | ||
| useTailwind, | ||
| } from '@metamask/design-system-twrnc-preset'; | ||
| import { | ||
| getElevatedSurfaceColor, | ||
| useElevatedSurface, | ||
| } from '../../../util/theme/themeUtils'; | ||
| import { BatchSellMetricsLocation } from '@metamask/bridge-controller'; | ||
| import { | ||
| useSafeAreaFrame, | ||
|
|
@@ -118,8 +113,6 @@ function TradeWalletActions() { | |
| const insetsTop = Platform.OS === 'android' ? insets.top : 0; | ||
|
|
||
| const tw = useTailwind(); | ||
| const surfaceClass = useElevatedSurface(); | ||
| const isPureBlack = usePureBlack(); | ||
| const theme = useTheme(); | ||
| const { colors } = theme; | ||
|
|
||
|
|
@@ -317,8 +310,6 @@ function TradeWalletActions() { | |
| [dismissRootModalFlow, exitingAnimationWithCallback], | ||
| ); | ||
|
|
||
| const elevatedSurfaceColor = getElevatedSurfaceColor(theme); | ||
|
|
||
| const actionList = ( | ||
| <> | ||
| {shouldRenderBatchSell && ( | ||
|
|
@@ -391,56 +382,23 @@ function TradeWalletActions() { | |
| <View style={tw.style('px-4')}> | ||
| <View | ||
| testID={WalletActionsBottomSheetSelectorsIDs.MENU_CONTAINER} | ||
| style={tw.style( | ||
| `${surfaceClass} p-4 rounded-t-2xl px-0`, | ||
| isPureBlack && 'border-t border-l border-r border-muted', | ||
| )} | ||
| style={tw.style('bg-default p-4 rounded-t-2xl px-0')} | ||
| > | ||
| {actionList} | ||
| </View> | ||
| <View | ||
| style={tw.style('flex-row mt-[-1px]', { height: bottomMaskHeight })} | ||
| > | ||
| <View | ||
| style={tw.style( | ||
| `${surfaceClass} flex-1 rounded-bl-2xl`, | ||
| isPureBlack && 'border-l border-b border-muted', | ||
| )} | ||
| /> | ||
| <View style={tw.style('bg-default flex-1 rounded-bl-2xl')} /> | ||
| <BottomShape | ||
| width={buttonLayout.width * 4} | ||
| height={bottomMaskHeight} | ||
| peakHeight={16} | ||
| peakBezierLength={25} | ||
| baseBezierLength={55} | ||
| fill={elevatedSurfaceColor} | ||
| fill={colors.background.default} | ||
| /> | ||
| <View | ||
| style={tw.style( | ||
| `${surfaceClass} flex-1 rounded-br-2xl`, | ||
| isPureBlack && 'border-r border-b border-muted', | ||
| )} | ||
| /> | ||
| {isPureBlack ? ( | ||
| <View | ||
| pointerEvents="none" | ||
| style={tw.style('absolute bottom-0 inset-x-0 items-center')} | ||
| testID={WalletActionsBottomSheetSelectorsIDs.MENU_BOTTOM_STROKE} | ||
| > | ||
| <BottomShape | ||
| width={buttonLayout.width * 4} | ||
| height={bottomMaskHeight} | ||
| peakHeight={16} | ||
| peakBezierLength={25} | ||
| baseBezierLength={55} | ||
| strokeOnly | ||
| pathProps={{ | ||
| stroke: colors.border.muted, | ||
| strokeWidth: 1, | ||
| }} | ||
| /> | ||
| </View> | ||
| ) : null} | ||
| <View style={tw.style('bg-default flex-1 rounded-br-2xl')} /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trade menu loses elevated surfaceLow Severity The custom trade actions sheet is not an MMDS Reviewed by Cursor Bugbot for commit 3fb3c08. Configure here. |
||
| </View> | ||
| </View> | ||
| </Animated.View> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import React, { useMemo } from 'react'; | ||
| import Svg, { Path, type PathProps } from 'react-native-svg'; | ||
| import Svg, { Path } from 'react-native-svg'; | ||
|
|
||
| function BottomShape({ | ||
| width, | ||
|
|
@@ -8,8 +8,6 @@ function BottomShape({ | |
| peakBezierLength = 25, | ||
| baseBezierLength = 55, | ||
| fill = 'black', | ||
| strokeOnly = false, | ||
| pathProps, | ||
| ...svgProps | ||
| }: { | ||
| width: number; | ||
|
|
@@ -18,8 +16,6 @@ function BottomShape({ | |
| peakBezierLength?: number; | ||
| baseBezierLength?: number; | ||
| fill?: string; | ||
| strokeOnly?: boolean; | ||
| pathProps?: PathProps; | ||
| }) { | ||
| const pathData = useMemo(() => { | ||
| const centerX = width / 2; | ||
|
|
@@ -32,25 +28,6 @@ function BottomShape({ | |
| const rightBaseX = centerX + baseBezierLength; | ||
| const rightBaseY = height; | ||
|
|
||
| // strokeOnly builds an open path for the border-muted stroke that traces the full | ||
| // bottom edge of the trade-menu shape: the flat shoulders on either side plus the | ||
| // cutout curve in the middle, so the border wraps continuously across the bottom. | ||
| // TradeWalletActions is the only caller; this branch can be removed if that menu no longer has a border. | ||
| if (strokeOnly) { | ||
| return ` | ||
| M ${width} ${height} | ||
| H ${rightBaseX} | ||
| C ${rightBaseX - peakBezierLength} ${rightBaseY} | ||
| ${peakX + peakBezierLength} ${peakY} | ||
| ${peakX} ${peakY} | ||
| S ${leftBaseX + peakBezierLength} ${leftBaseY} | ||
| ${leftBaseX} ${leftBaseY} | ||
| H 0 | ||
| ` | ||
| .replace(/\s+/g, ' ') | ||
| .trim(); | ||
| } | ||
|
|
||
| return ` | ||
| M 0 ${height} | ||
| V 0 | ||
|
|
@@ -67,18 +44,11 @@ function BottomShape({ | |
| ` | ||
| .replace(/\s+/g, ' ') | ||
| .trim(); | ||
| }, [ | ||
| width, | ||
| height, | ||
| peakHeight, | ||
| peakBezierLength, | ||
| baseBezierLength, | ||
| strokeOnly, | ||
| ]); | ||
| }, [width, height, peakHeight, peakBezierLength, baseBezierLength]); | ||
|
|
||
| return ( | ||
| <Svg width={width} height={height} {...svgProps}> | ||
| <Path d={pathData} fill={strokeOnly ? 'none' : fill} {...pathProps} /> | ||
| <Path d={pathData} fill={fill} /> | ||
| </Svg> | ||
| ); | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cursoragent revert the changes to this file
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||




There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MMDS BottomSheet callsites no longer need twClassName={surfaceClass} because BottomSheetDialog owns elevated surface styling since design-system-react-native 0.33. CustomNetworkSelector and NetworkMultiSelectorList instead drop per-row surfaceClass that duplicated the parent sheet background. AddressSelector and DeleteNetworkModal match the BottomSheet pattern in this PR.