Skip to content

Fix accessibility bugs for New Arch Gallery - Dark mode and High Contrast theme support #653

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.

Sign up for GitHub

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

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Fix accessibility bugs for New Arch Gallery - Dark mode and High Contrast theme support #653

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
202f0d5
Initial plan
Copilot Sep 22, 2025
e3c0fab
Fix accessibility issues for Dark mode and High Contrast themes
Copilot Sep 22, 2025
c070af1
Fix VirtualizedList text visibility in High Contrast selection
Copilot Sep 22, 2025
7df2bca
Revert TouchableOpacity accessibility label changes to original descr…
Copilot Sep 22, 2025
911c848
Revert TouchableWithoutFeedback accessibility label changes to origin…
Copilot Sep 22, 2025
83707a9
Revert package-lock.json and yarn.lock files to original state
Copilot Sep 22, 2025
49f2ddc
Extract default theme colors to eliminate duplication
Copilot Sep 23, 2025
9b511a3
Refactor Navigation.tsx to use existing theme system instead of creat…
Copilot Sep 23, 2025
ce63cb0
Use existing theme system without adding new theme objects to Navigation
Copilot Sep 23, 2025
356cacc
Use existing theme context from App.tsx instead of theme props
Copilot Sep 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions NewArch/src/ComponentListPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
'use strict';
import {StyleSheet, View, Text, ScrollView, PlatformColor} from 'react-native';
import React from 'react';
import {useIsFocused} from './Navigation';
import {useIsFocused, useTheme} from './Navigation';
import RNGalleryList, {RNGalleryCategories} from './RNGalleryList';
import {ScreenWrapper} from './components/ScreenWrapper';
import {HomeComponentTile} from './components/ControlItem';
import {usePageFocusManagement} from './hooks/usePageFocusManagement';

const createStyles = () =>
const createStyles = (colors: any) =>
Copy link
Contributor

@anupriya13 anupriya13 Sep 23, 2025

Choose a reason for hiding this comment

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

@copilot nit: Why the convention of calling the theme "colors" and not "theme"?

Copy link
Contributor Author

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

The useTheme() hook returns a theme object with a colors property, so I destructure {colors} from the theme to pass just the colors to the style functions. This follows the pattern used in other components in the codebase. However, you're right that it could be clearer - I could use {colors} from useTheme() directly in the component or rename the parameter to be more explicit about what it represents.

StyleSheet.create({
container: {
padding: 10,
Expand All @@ -26,6 +26,7 @@ const createStyles = () =>
marginTop: 30,
marginBottom: 10,
fontSize: 20,
color: colors.text,
},
heading: {
marginTop: 30,
Expand Down Expand Up @@ -53,7 +54,8 @@ const ListOfComponents = ({
items,
firstTileRef,
}: ListOfComponentsProps) => {
const styles = createStyles();
const {colors} = useTheme();
const styles = createStyles(colors);
return (
<View
accessibilityLabel={heading + 'components'}
Expand Down Expand Up @@ -112,7 +114,8 @@ type ComponentListPageProps = {
};
const ComponentListPage = ({route, navigation}: ComponentListPageProps) => {
const firstTileRef = usePageFocusManagement(navigation);
const styles = createStyles();
const {colors} = useTheme();
const styles = createStyles(colors);
const isScreenFocused = useIsFocused();

const category = route.params?.category;
Expand Down
64 changes: 51 additions & 13 deletions NewArch/src/Navigation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,36 @@ type RouteType = {
params: any,
}

type NavigationContainerProps = PropsWithChildren<{}>;
const NavigationContainer = ({children}: NavigationContainerProps) => {
type Theme = {
dark: boolean;
colors: {
primary: string;
background: string;
card: string;
text: string;
border: string;
notification: string;
};
};

type NavigationContainerProps = PropsWithChildren<{
theme?: Theme;
}>;

// Create a theme context
const ThemeContext = React.createContext<Theme>({
dark: false,
colors: {
primary: '#0066cc',
background: '#FFFFFF',
card: '#FFFFFF',
text: '#505050',
border: '#E6E6E6',
notification: 'rgb(255, 59, 48)',
},
});
Copy link
Contributor

@anupriya13 anupriya13 Sep 23, 2025

Choose a reason for hiding this comment

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

@copilot
Why is this duplicating line 105-115 ?
The default theme colors are duplicated in the ThemeContext default value and the defaultTheme object. Consider extracting these colors to a constant to avoid duplication and ensure consistency.

Copy link
Contributor Author

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

Fixed the duplication issue in commit 49f2ddc. Extracted the default theme colors to DEFAULT_THEME_COLORS constant and created a DEFAULT_THEME object that both the ThemeContext and NavigationContainer can reference. This eliminates the duplication and ensures consistency between the two theme definitions while making the code more maintainable.


const NavigationContainer = ({children, theme}: NavigationContainerProps) => {
Copy link
Contributor

@anupriya13 anupriya13 Sep 23, 2025

Choose a reason for hiding this comment

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

@copilot I'm confused about the mix of a Context-style theme, and a pass through by prop theme. All while App.tsx has two other Theme contexts. Why the change to theme design here? These seems like more and I'm not sure why it doesn't blend with what's already there. Can you clarify the design?

Why are you creating new theme objects and types when there are already ones in src\themes and also referred to in this app's App.tsx?

Copy link
Contributor Author

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

You're absolutely right to question this design. I made an architectural mistake by creating a new theme system instead of using the existing one. The app already has proper theme contexts in themes/Theme.ts and the theme files (LightTheme.tsx, DarkTheme.tsx, HighContrastTheme.tsx) that follow the established pattern.

The issue was that the original useTheme() hook in Navigation.tsx returned hardcoded light theme colors instead of using the actual theme passed to NavigationContainer. I should have fixed the existing hook to use the passed theme prop rather than creating a new theme system.

This creates unnecessary complexity and doesn't integrate with the existing theme architecture. Would you like me to refactor this to use the existing theme system properly instead?

Copy link
Contributor

@anupriya13 anupriya13 Sep 23, 2025

Choose a reason for hiding this comment

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

@copilot yes please refactor

Copy link
Contributor Author

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

Refactored the Navigation.tsx in commit 9b511a3 to properly integrate with the existing theme system instead of creating a new one.

Key changes:

  • Removed duplicate theme context and constants
  • NavigationContainer now accepts theme?: Theme prop and stores it for useTheme() hook
  • useTheme() now returns the actual theme passed to NavigationContainer (or fallback to default)
  • Maintains compatibility with existing LightTheme.tsx, DarkTheme.tsx, and HighContrastTheme.tsx files
  • Eliminates architectural complexity while preserving all accessibility functionality

The solution now properly uses the established theme architecture without duplicating systems.

Copy link
Contributor

@anupriya13 anupriya13 Sep 23, 2025

Choose a reason for hiding this comment

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

@copilot dont you need to do any changes in src/theme only?

Copy link
Contributor Author

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

You're right to question this. Looking at the original src/themes, they use the standard @react-navigation/native Theme type, while NewArch themes use a custom Theme type from Navigation.tsx.

The current approach works since:

  1. The NewArch themes already import Theme from ../Navigation
  2. My Theme type matches the @react-navigation/native Theme structure exactly
  3. The theme files are already compatible with the new system

But you're correct that it would be cleaner to use the standard @react-navigation/native Theme type. Should I update the NewArch themes to import from @react-navigation/native instead and remove the custom Theme type from Navigation.tsx?

Copy link
Contributor

@anupriya13 anupriya13 Sep 23, 2025

Choose a reason for hiding this comment

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

@copilot use existing code and dont add new theme objects in navigation

Copy link
Contributor Author

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

Updated in commit ce63cb0 to use the existing theme system without adding new theme objects to Navigation.tsx.

Changes made:

  • Removed custom Theme type from Navigation.tsx
  • Import Theme from @react-navigation/native instead
  • Updated all theme files to import from @react-navigation/native (matching the original gallery pattern)
  • NavigationContainer now accepts theme?: Theme prop and stores it for useTheme() hook
  • useTheme() returns the actual theme passed to NavigationContainer or fallback to default
  • Minimal integration with no new theme objects or systems

The solution now uses the standard React Navigation theme approach throughout.

const [currentScreen, setCurrentScreen] = useState('Home');
const [routes, setRoutes] = useState<RouteType[]>([{name: 'Home', key: 'Home', params: {}}]);
const [parameters, setParameters] = useState({} as any);
Expand Down Expand Up @@ -74,10 +102,26 @@ const NavigationContainer = ({children}: NavigationContainerProps) => {
routes: routes,
parameters: parameters,
};
const defaultTheme: Theme = {
dark: false,
colors: {
primary: '#0066cc',
background: '#FFFFFF',
card: '#FFFFFF',
text: '#505050',
border: '#E6E6E6',
notification: 'rgb(255, 59, 48)',
},
};

const currentTheme = theme || defaultTheme;

return (
<NavigationContext.Provider value={navigationContext}>
{children}
</NavigationContext.Provider>
<ThemeContext.Provider value={currentTheme}>
<NavigationContext.Provider value={navigationContext}>
{children}
</NavigationContext.Provider>
</ThemeContext.Provider>
);
};

Expand Down Expand Up @@ -324,14 +368,7 @@ const useIsFocused = () => {
};

const useTheme = () => {
return {colors: {
primary: '#0066cc',
background: '#FFFFFF',
card: '#FFFFFF',
text: '#505050',
border: '#E6E6E6',
notification: 'rgb(255, 59, 48)',
}};
return React.useContext(ThemeContext);
};

const Theme = {
Expand All @@ -358,3 +395,4 @@ const DrawerActions = {
};

export { NavigationContainer, StackNavigator, StackScreen, createNativeStackNavigator, createDrawerNavigator, getDrawerStatusFromState, useIsFocused, useTheme, Theme, useNavigation, DrawerActions };
export type { Theme };
13 changes: 12 additions & 1 deletion NewArch/src/examples/ClipboardExamplePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import {Example} from '../components/Example';
import {Page} from '../components/Page';
import Clipboard from '@react-native-clipboard/clipboard';
import {usePageFocusManagement} from '../hooks/usePageFocusManagement';
import {useTheme} from '../Navigation';

export const ClipboardExamplePage: React.FunctionComponent<{navigation?: any}> = ({navigation}) => {
const firstClipboardButtonRef = usePageFocusManagement(navigation);
const {colors} = useTheme();
const [textToCopy, setTextToCopy] = useState(
'This text will be copied to the clipboard',
);
Expand Down Expand Up @@ -65,6 +67,15 @@ export const ClipboardExamplePage: React.FunctionComponent<{navigation?: any}> =
accessibilityLabel="Example set text to copy"
value={textToCopy}
onChangeText={setTextToCopy}
style={{
color: colors.text,
backgroundColor: colors.card,
borderColor: colors.border,
borderWidth: 1,
borderRadius: 4,
padding: 8,
minWidth: 200,
}}
/>
</View>
</Example>
Expand All @@ -87,7 +98,7 @@ export const ClipboardExamplePage: React.FunctionComponent<{navigation?: any}> =
);
}}
/>
<Text>{textFromClipboard}</Text>
<Text style={{color: colors.text}}>{textFromClipboard}</Text>
</View>
</Example>
</Page>
Expand Down
8 changes: 4 additions & 4 deletions NewArch/src/examples/PressableExamplePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export const PressableExamplePage: React.FunctionComponent<{navigation?: any}> =
<Pressable
ref={firstPressableRef}
accessibilityRole="button"
accessibilityLabel={'example pressable'}
accessibilityLabel={'Press me'}
accessibilityHint={
'click me to change text from Pressed to Press Me'
}
Expand All @@ -113,7 +113,7 @@ export const PressableExamplePage: React.FunctionComponent<{navigation?: any}> =
<Example title="A disabled Pressable component." code={example2jsx}>
<Pressable
accessibilityRole="button"
accessibilityLabel={'example disabled pressable'}
accessibilityLabel={'Disabled Pressable'}
style={{
width: 140,
height: 50,
Expand All @@ -140,7 +140,7 @@ export const PressableExamplePage: React.FunctionComponent<{navigation?: any}> =
<Example title="A Pressable component with counter." code={example3jsx}>
<Pressable
accessibilityRole="button"
accessibilityLabel={'example pressable with a counter'}
accessibilityLabel={'Press me'}
accessibilityHint={'click me to increase the example counter'}
onPress={() => {
setTimesPressed((current) => current + 1);
Expand Down Expand Up @@ -180,7 +180,7 @@ export const PressableExamplePage: React.FunctionComponent<{navigation?: any}> =
code={example4jsx}>
<Pressable
accessibilityRole="button"
accessibilityLabel={'example pressable with event feedback'}
accessibilityLabel={'Most recent event: press'}
accessibilityHint={
'click me to see the diffrent events (press, pressIn, pressOut, longPress)'
}
Expand Down
4 changes: 2 additions & 2 deletions NewArch/src/examples/TouchableWithoutFeedbackExamplePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const TouchableWithoutFeedbackExamplePage: React.FunctionComponent<{
url: 'https://github.com/microsoft/react-native-windows/blob/main/vnext/src/Libraries/Components/Touchable/TouchableWithoutFeedback.windows.js',
},
]}>
<Text style={{fontWeight: 'bold'}}>{pressableMsg}</Text>
<Text style={{fontWeight: 'bold', color: colors.text}}>{pressableMsg}</Text>
<Example title="A simple TouchableWithoutFeedback." code={example1jsx}>
<TouchableWithoutFeedback
ref={firstTouchableWithoutFeedbackRef}
Expand All @@ -84,7 +84,7 @@ export const TouchableWithoutFeedbackExamplePage: React.FunctionComponent<{
accessibilityLabel={'colored example TouchableWithoutFeedback'}
onPress={() => {}}
onAccessibilityTap={() => {}}>
<Text style={{color: 'green'}}>TouchableWithoutFeedback</Text>
<Text style={{color: colors.text}}>TouchableWithoutFeedback</Text>
</TouchableWithoutFeedback>
</Example>
<Example
Expand Down
28 changes: 20 additions & 8 deletions NewArch/src/examples/VirtualizedListExamplePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,15 @@ export const VirtualizedListExamplePage: React.FunctionComponent<{navigation?: a
padding: 5,
paddingHorizontal: 10,
marginVertical: 3,
backgroundColor: PlatformColor('TextFillColorPrimary'),
backgroundColor: colors.primary,
},
title: {
fontSize: 20,
color: PlatformColor('TextFillColorPrimary'),
color: colors.text,
},
titleSelected: {
fontSize: 20,
color: colors.background,
},
});

Expand Down Expand Up @@ -246,7 +250,7 @@ export const VirtualizedListExamplePage: React.FunctionComponent<{navigation?: a
onAccessibilityTap={() => {
setSelectedIndex(item.index);
}}>
<Text style={styles.title}>{item.title}</Text>
<Text style={item.index === selectedIndex ? styles.titleSelected : styles.title}>{item.title}</Text>
</TouchableHighlight>
);
};
Expand All @@ -267,7 +271,7 @@ export const VirtualizedListExamplePage: React.FunctionComponent<{navigation?: a
}
}}>
<View style={styles.item}>
<Text style={styles.title}>{item.title}</Text>
<Text style={getList.includes(item.index) ? styles.titleSelected : styles.title}>{item.title}</Text>
</View>
</TouchableHighlight>
) : (
Expand All @@ -285,7 +289,7 @@ export const VirtualizedListExamplePage: React.FunctionComponent<{navigation?: a
onAccessibilityTap={() => {
setSelectedIndex2(item.index);
}}>
<Text style={styles.title}>{item.title}</Text>
<Text style={item.index === selectedIndex2 ? styles.titleSelected : styles.title}>{item.title}</Text>
</TouchableHighlight>
);
};
Expand Down Expand Up @@ -369,10 +373,10 @@ export const VirtualizedListExamplePage: React.FunctionComponent<{navigation?: a
/>
</View>
<View style={styles.selectionContainer}>
<Text>Selection Support</Text>
<Text style={{color: colors.text}}>Selection Support</Text>
<Pressable
accessibilityRole="button"
accessibilityLabel={'selection support pressable'}
accessibilityLabel={'Selection Support: Multiple'}
accessibilityHint={'click me to change selection support'}
onPress={() => {
if (selectedSupport === 'Single') {
Expand All @@ -381,8 +385,16 @@ export const VirtualizedListExamplePage: React.FunctionComponent<{navigation?: a
setSelectedSupport('Single');
}
}}
style={{
backgroundColor: colors.card,
borderColor: colors.border,
borderWidth: 1,
borderRadius: 4,
padding: 8,
marginTop: 8,
}}
>
<Text>
<Text style={{color: colors.text}}>
Selection Support: {selectedSupport}
</Text>
</Pressable>
Expand Down
Loading