Skip to content

FIX: dark mode theme, update alert dialogs, and UI consistency improvements#72

Open
chaitika wants to merge 2 commits into
CypherCommons:masterfrom
chaitika:update-ui
Open

FIX: dark mode theme, update alert dialogs, and UI consistency improvements#72
chaitika wants to merge 2 commits into
CypherCommons:masterfrom
chaitika:update-ui

Conversation

@chaitika
Copy link
Copy Markdown
Contributor

@chaitika chaitika commented Mar 11, 2026

This PR:

  • restores dynamic dark/light theme switching based on device setting
  • add custom alert modal
  • updates dark mode theme as per brand aesthetic
  • migrates DeleteWallet from raw Alert.alert() to presentAlert wrapper
  • updates loading spinner color as per brand aesthetic
  • fixs ImportWallet screen's half-black background in dark mode

View Screenshots at Image log for PR #72

Copilot AI review requested due to automatic review settings March 11, 2026 09:13
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR focuses on dark-mode/light-mode behavior and UI consistency by updating theme selection at the app level, introducing a custom alert modal, and aligning several UI elements with updated brand colors.

Changes:

  • Switch NavigationContainer theme dynamically based on device color scheme.
  • Add a CustomAlert modal and wire it into the existing presentAlert wrapper (Android path).
  • Update wallet deletion confirmation copy and a few UI elements (spinners, ImportWallet layout).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
App.tsx Uses useColorScheme() to select dark/light theme; wires CustomAlert ref into presentAlert.
components/Alert.ts Adds a global customAlertRef and routes Android alerts through CustomAlert when available.
components/CustomAlert.tsx New custom modal implementation that renders title/message/buttons styled by theme.
components/themes.ts Updates dark theme button color and adds brand color entries.
loc/en.json Updates wallet deletion-related strings.
screen/settings/DeleteWallet.tsx Migrates deletion prompts from Alert.alert() to presentAlert.
screen/wallets/ImportWallet.tsx Adjusts layout styling to fix dark-mode background issue.
screen/UnlockWith.tsx Updates spinner color to match brand color.
navigation/LazyLoadingIndicator.tsx Updates spinner color to match brand color.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread loc/en.json Outdated
Comment thread screen/settings/DeleteWallet.tsx Outdated
Comment thread App.tsx
Comment thread App.tsx Outdated
Comment thread components/Alert.ts Outdated
Comment thread screen/settings/DeleteWallet.tsx Outdated
Comment thread components/CustomAlert.tsx
Comment thread components/CustomAlert.tsx Outdated
Comment thread loc/en.json Outdated
export const LazyLoadingIndicator = () => (
<View style={styles.root}>
<ActivityIndicator size="large" />
<ActivityIndicator size="large" color="#754CE8" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't hard-code colors

Comment thread loc/en.json
"details_del_wb_q": "This wallet has a balance. Before proceeding, please be aware that you will not be able to recover the funds without this wallet’s seed phrase. In order to avoid accidental removal, please enter your wallet’s balance of {balance} satoshis.",
"details_delete": "Delete",
"details_delete_wallet": "Delete Wallet",
"details_delete_wallet": "⚠️ Delete Wallet?",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get rid of the emoji here. localization files carry text context

Comment on lines +16 to +17
const isDark = Appearance.getColorScheme() === 'dark';
const colors = isDark ? BlueDarkTheme.colors : BlueDefaultTheme.colors;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of Appearance.getColorScheme - which just takes a snapshot, we should use useColorScheme.

Suggested change
const isDark = Appearance.getColorScheme() === 'dark';
const colors = isDark ? BlueDarkTheme.colors : BlueDefaultTheme.colors;
const colorScheme = useColorScheme();
const colors = colorScheme === 'dark' ? BlueDarkTheme.colors : BlueDefaultTheme.colors;

Comment thread App.tsx
Comment on lines +17 to +26
const customAlertRef = React.useRef<CustomAlertHandle>(null);

React.useEffect(() => {
setCustomAlertRef(customAlertRef.current);
initializeIndexer({
baseUrl: 'https://cushionlike-isabel-retrievable.ngrok-free.dev/',
timeout: 100000, // 100 seconds for blockchain scanning operations (increased for slower connections)
});
return () => setCustomAlertRef(null);
}, []);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont like the approach of using useRef inside a useEffect

Comment thread App.tsx
Comment on lines +17 to +26
const customAlertRef = React.useRef<CustomAlertHandle>(null);

React.useEffect(() => {
setCustomAlertRef(customAlertRef.current);
initializeIndexer({
baseUrl: 'https://cushionlike-isabel-retrievable.ngrok-free.dev/',
timeout: 100000, // 100 seconds for blockchain scanning operations (increased for slower connections)
});
return () => setCustomAlertRef(null);
}, []);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const customAlertRef = React.useRef<CustomAlertHandle>(null);
React.useEffect(() => {
setCustomAlertRef(customAlertRef.current);
initializeIndexer({
baseUrl: 'https://cushionlike-isabel-retrievable.ngrok-free.dev/',
timeout: 100000, // 100 seconds for blockchain scanning operations (increased for slower connections)
});
return () => setCustomAlertRef(null);
}, []);
const handleCustomAlertRef = React.useCallback((ref: CustomAlertHandle | null) => {
setCustomAlertRef(ref);
}, []);
React.useEffect(() => {
initializeIndexer({
baseUrl: 'https://cushionlike-isabel-retrievable.ngrok-free.dev/',
timeout: 100000, // 100 seconds for blockchain scanning operations (increased for slower connections)
});
}, []);

Comment thread App.tsx
Comment on lines +30 to +32
React.useEffect(() => {
BlueCurrentTheme.updateColorScheme();
}, [colorScheme]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dead code

Comment thread App.tsx
</StorageProvider>
</SafeAreaProvider>
</NavigationContainer>
<CustomAlert ref={customAlertRef} />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be mounted once at the app root level and can work globally across screens, please move this to just below <MasterView />

[dismiss],
);

const getButtonTextStyle = (style?: string) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const getButtonTextStyle = (style?: string) => {
const getButtonTextStyle = (style?: AlertButton['style']) => {

[
presentAlert({
title: loc.wallets.details_delete_wallet,
message: loc.wallets.delete_wallet_warning,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the message should be a combination of both, are you sure + you will need your seed phrase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants