fix: use background/section for bottom sheet in dark mode#28609
fix: use background/section for bottom sheet in dark mode#28609georgewrmarshall wants to merge 7 commits intomainfrom
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
metamaskbot
added
the
team-design-system
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: PermissionsSummary loses background when rendered full-screen
- Added conditional background.alternative to safeArea and mainContainer when not rendered as a BottomSheet to restore full-screen background.
Or push these changes by commenting:
@cursor push f21433a123
Preview (f21433a123)
diff --git a/app/components/UI/PermissionsSummary/PermissionsSummary.styles.ts b/app/components/UI/PermissionsSummary/PermissionsSummary.styles.ts
--- a/app/components/UI/PermissionsSummary/PermissionsSummary.styles.ts
+++ b/app/components/UI/PermissionsSummary/PermissionsSummary.styles.ts
@@ -18,12 +18,23 @@
const height = nonTabView ? tabHeight : bottomSheetHeight;
return StyleSheet.create({
- safeArea: {},
+ safeArea: {
+ // Provide background only when rendered as a full-screen view.
+ // BottomSheet supplies its own background.
+ backgroundColor: isRenderedAsBottomSheet
+ ? undefined
+ : colors.background.alternative,
+ },
mainContainer: {
paddingTop: 16,
borderTopLeftRadius: 20,
borderTopRightRadius: 20,
height,
+ // Provide background only when rendered as a full-screen view.
+ // BottomSheet supplies its own background.
+ backgroundColor: isRenderedAsBottomSheet
+ ? undefined
+ : colors.background.alternative,
justifyContent: isRenderedAsBottomSheet ? 'flex-start' : 'space-between',
},
contentContainer: {You can send follow-ups to the cloud agent here.
georgewrmarshall
force-pushed
the
bottomsheet-color-change-clean
branch
from
14723a5 to
1b4f862
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Unused
colorsdestructuring after removing its only usage- Removed the unused
colorsdestructuring from LearnMoreBottomSheet.styles.ts.
- Removed the unused
Or push these changes by commenting:
@cursor push 7f0aed32cc
Preview (7f0aed32cc)
diff --git a/app/components/Views/MultichainAccounts/IntroModal/LearnMoreBottomSheet.styles.ts b/app/components/Views/MultichainAccounts/IntroModal/LearnMoreBottomSheet.styles.ts
--- a/app/components/Views/MultichainAccounts/IntroModal/LearnMoreBottomSheet.styles.ts
+++ b/app/components/Views/MultichainAccounts/IntroModal/LearnMoreBottomSheet.styles.ts
@@ -3,7 +3,6 @@
const styleSheet = (params: { theme: Theme }) => {
const { theme } = params;
- const { colors } = theme;
return StyleSheet.create({
container: {},You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 1b4f862. Configure here.
app/components/Views/MultichainAccounts/IntroModal/LearnMoreBottomSheet.styles.ts
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #28609 +/- ##
===========================================
- Coverage 82.13% 65.39% -16.75%
===========================================
Files 4942 4942
Lines 129947 129966 +19
Branches 28977 28984 +7
===========================================
- Hits 106732 84989 -21743
- Misses 15917 38381 +22464
+ Partials 7298 6596 -702 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
georgewrmarshall
force-pushed
the
bottomsheet-color-change-clean
branch
2 times, most recently
from
fe752e9 to
e172e70
Compare
- Update BottomSheetDialog to use background.section in dark mode and background.default in light mode - Apply same conditional logic to SheetHeader for consistency - Remove redundant full-bleed backgroundColor overrides from PermissionsSummary, DeepLinkModal, LearnMoreBottomSheet, and NetworkVerificationInfo so sheet background shows through - Remove bg-default from QRAccountDisplay root box so sheet background shows through
| backgroundColor: | ||
| themeAppearance === AppThemeKey.dark | ||
| ? colors.background.section | ||
| : colors.background.default, |
There was a problem hiding this comment.
The bottom sheet shell now owns the background color, with a theme-aware split: dark mode gets background.section (a slightly elevated surface) while light mode keeps background.default. Centralizing this here means child components no longer need to redeclare a background color — the sheet itself is the single source of truth.
| backgroundColor: | ||
| themeAppearance === AppThemeKey.dark | ||
| ? colors.background.section | ||
| : colors.background.default, |
There was a problem hiding this comment.
The sheet header mirrors the same dark/light split as the dialog container so its background stays visually flush with the sheet behind it. Without this, the header would show background.default on top of a background.section sheet in dark mode, creating an unintended contrast stripe.
| backgroundColor: theme.colors.background.alternative, | ||
| ...(!isRenderedAsBottomSheet && { | ||
| backgroundColor: theme.colors.background.alternative, | ||
| }), |
There was a problem hiding this comment.
PermissionsSummary is used in two contexts: as a full-screen page and as a bottom sheet. In full-screen mode the component owns its background (background.alternative), but when rendered inside a BottomSheet it should be transparent so the sheet's own background shows through. The conditional spread achieves this without needing a separate style variant.
| @@ -12,7 +12,6 @@ const styleSheet = (params: { theme: Theme }) => { | |||
|
|
|||
| return StyleSheet.create({ | |||
There was a problem hiding this comment.
Removing the explicit background.default here lets the parent BottomSheetDialog surface its correctly themed background. Previously this hardcoded value would override the sheet's dark-mode background.section color, producing a mismatched inner panel in dark theme.
|
|
||
| return ( | ||
| <Box twClassName="bg-default items-center"> | ||
| <Box twClassName="items-center"> |
There was a problem hiding this comment.
The bg-default Tailwind class was hardcoding a light-mode white background on the QR display container. Removing it allows the parent bottom sheet to provide the correct background for both themes rather than having this component override it.
georgewrmarshall
force-pushed
the
bottomsheet-color-change-clean
branch
from
e172e70 to
c57e411
Compare
There was a problem hiding this comment.
After — QRAccountDisplay
Light
🔍 Smart E2E Test Selection⏭️ Smart E2E selection skipped - draft PR All E2E tests pre-selected. |
There was a problem hiding this comment.
After — NetworkVerificationInfo
There was a problem hiding this comment.
After — LearnMoreBottomSheet
| flex: 1, | ||
| paddingHorizontal: 16, | ||
| alignItems: 'center', | ||
| backgroundColor: colors.background.default, |
There was a problem hiding this comment.
Can't render the UI here but confident in the code changes when looking at where this color is applied
|
✅ E2E Fixture Validation — Schema is up to date |
|
| ...(!isRenderedAsBottomSheet && { | ||
| backgroundColor: theme.colors.background.alternative, | ||
| }), | ||
| paddingTop: 16, |
3 participants
Description
Updates the BottomSheet background color to improve dark mode visual hierarchy. Previously, all bottom sheets used
background/defaultin both light and dark modes. This change usesbackground/sectionin dark mode to visually elevate the sheet surface above the page background.Also removes redundant full-bleed
backgroundColoroverrides from content components rendered inside BottomSheets, which were conflicting with the sheet's background.Changelog
CHANGELOG entry: null
Related issues
Fixes:
Manual testing steps
```gherkin
Feature: BottomSheet dark mode background
Scenario: user opens any bottom sheet in dark mode
Given the app is in dark mode
When user triggers any bottom sheet (e.g. account options, network switcher, permissions)
Then the sheet surface should appear elevated using background/section
And content inside the sheet should not override the sheet background
Scenario: user opens any bottom sheet in light mode
Given the app is in light mode
When user triggers any bottom sheet
Then the sheet background should remain background/default (no visual change)
```
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Low Risk
Low risk visual-only changes to bottom sheet and sheet header backgrounds plus removal of nested content background overrides; main risk is unintended contrast/regression in dark-mode styling across affected screens.
Overview
Improves dark-mode visual hierarchy by switching
BottomSheetDialogandSheetHeadersurfaces tocolors.background.sectionwhenthemeAppearanceis dark (keepingbackground.defaultin light mode).Removes/conditions full-bleed
backgroundColorstyles inside components commonly rendered within bottom sheets (e.g.,DeepLinkModal,NetworkVerificationInfo,PermissionsSummary,QRAccountDisplay, and the multichain learn-more sheet) so the sheet surface can show through; updates Jest snapshots accordingly.Reviewed by Cursor Bugbot for commit c57e411. Bugbot is set up for automated code reviews on this repo. Configure here.