Skip to content

Commit 8fbf2fa

Browse files
chiciometa-codesync[bot]
authored andcommitted
Fix AccessibilityInfo.isDarkerSystemColorsEnabled unresolved promise (facebook#56019)
Summary: This PR fixes a bug in the `AccessibilityInfo` component. This is a follow up of the work I started with the PR I opened last week that has been already merged facebook#55920. `AccessibilityInfo` exposes the method `isDarkerSystemColorsEnabled`. In the android branch, Promise.resolve(false) as return value was creating a new unrelated promise that never calls the resolve function provided by the promise executor constructor, so it hangs indefinitely. This means that if a user calls this method on android without any `Platform.OS === 'ios'` guard, it will result in a unresolved promise. I fixed the bug in the same way I fixed the one in the PR above: by aligning the implementation of `isDarkerSystemColorsEnabled` to the one of other methods (eg. `isBoldTextEnabled`). I also added the tests to avoid regression, and additionally verify that the iOS method is doing what we are expecting (in both cases for when `NativeAccessibilityManagerIOS.getCurrentDarkerSystemColorsState` is available or not). The mock of the Platform object has been done the same way I saw while doing another contribution in the Pressability-test (see facebook#55378 ). ## Changelog: [GENERAL] [FIXED] - Fix AccessibilityInfo.isDarkerSystemColorsEnabled unresolved (never ending) promise Pull Request resolved: facebook#56019 Test Plan: This fix, like the one in the previous PR, was develop using TDD. I first added a failing test to reproduce that the android branch of the `isDarkerSystemColorsEnabled` method was acting as described above (resulting in failure due to jest timeout because the promise was not returning). Then I applied the fix, and finally added also the test for the iOS counterpart. Reviewed By: cortinico Differential Revision: D95931847 Pulled By: vzaidman fbshipit-source-id: 94cd088b286f1b724f0feeb20670b501a458c774
1 parent f1ac82c commit 8fbf2fa

2 files changed

Lines changed: 66 additions & 6 deletions

File tree

packages/react-native/Libraries/Components/AccessibilityInfo/AccessibilityInfo.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -237,10 +237,10 @@ const AccessibilityInfo = {
237237
* The result is `true` when dark system colors is enabled and `false` otherwise.
238238
*/
239239
isDarkerSystemColorsEnabled(): Promise<boolean> {
240-
return new Promise((resolve, reject) => {
241-
if (Platform.OS === 'android') {
242-
return Promise.resolve(false);
243-
} else {
240+
if (Platform.OS === 'android') {
241+
return Promise.resolve(false);
242+
} else {
243+
return new Promise((resolve, reject) => {
244244
if (
245245
NativeAccessibilityManagerIOS?.getCurrentDarkerSystemColorsState !=
246246
null
@@ -256,8 +256,8 @@ const AccessibilityInfo = {
256256
),
257257
);
258258
}
259-
}
260-
});
259+
});
260+
}
261261
},
262262

263263
/**

packages/react-native/Libraries/Components/AccessibilityInfo/__tests__/AccessibilityInfo-test.js

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ jest.unmock('../AccessibilityInfo');
1313
const mockGetCurrentPrefersCrossFadeTransitionsState = jest.fn(
1414
(onSuccess, onError) => onSuccess(true),
1515
);
16+
const mockGetCurrentDarkerSystemColorsState = jest.fn((onSuccess, onError) =>
17+
onSuccess(true),
18+
);
1619
const mockNativeAccessibilityManagerDefault: {
1720
getCurrentPrefersCrossFadeTransitionsState: JestMockFn<
1821
[
@@ -21,9 +24,17 @@ const mockNativeAccessibilityManagerDefault: {
2124
],
2225
void,
2326
> | null,
27+
getCurrentDarkerSystemColorsState: JestMockFn<
28+
[
29+
onSuccess: (isDarkerSystemColorsEnabled: boolean) => void,
30+
onError: (error: Error) => void,
31+
],
32+
void,
33+
> | null,
2434
} = {
2535
getCurrentPrefersCrossFadeTransitionsState:
2636
mockGetCurrentPrefersCrossFadeTransitionsState,
37+
getCurrentDarkerSystemColorsState: mockGetCurrentDarkerSystemColorsState,
2738
};
2839

2940
jest.mock('../NativeAccessibilityManager', () => ({
@@ -41,6 +52,7 @@ describe('AccessibilityInfo', () => {
4152
beforeEach(() => {
4253
originalPlatform = Platform.OS;
4354
mockGetCurrentPrefersCrossFadeTransitionsState.mockClear();
55+
mockGetCurrentDarkerSystemColorsState.mockClear();
4456
});
4557

4658
describe('prefersCrossFadeTransitions', () => {
@@ -91,9 +103,57 @@ describe('AccessibilityInfo', () => {
91103
});
92104
});
93105

106+
describe('isDarkerSystemColorsEnabled', () => {
107+
describe('Android', () => {
108+
it('should return immediately', async () => {
109+
/* $FlowFixMe[incompatible-type] */
110+
Platform.OS = 'android';
111+
112+
const isDarkerSystemColorsEnabled =
113+
await AccessibilityInfo.isDarkerSystemColorsEnabled();
114+
115+
expect(isDarkerSystemColorsEnabled).toBe(false);
116+
});
117+
});
118+
119+
describe('iOS', () => {
120+
it('should call getCurrentDarkerSystemColorsState if available', async () => {
121+
/* $FlowFixMe[incompatible-type] */
122+
Platform.OS = 'ios';
123+
124+
const isDarkerSystemColorsEnabled =
125+
await AccessibilityInfo.isDarkerSystemColorsEnabled();
126+
127+
expect(mockGetCurrentDarkerSystemColorsState).toHaveBeenCalled();
128+
expect(isDarkerSystemColorsEnabled).toBe(true);
129+
});
130+
131+
it('should throw error if getCurrentDarkerSystemColorsState is not available', async () => {
132+
/* $FlowFixMe[incompatible-type] */
133+
Platform.OS = 'ios';
134+
135+
mockNativeAccessibilityManagerDefault.getCurrentDarkerSystemColorsState =
136+
null;
137+
138+
const result: mixed =
139+
await AccessibilityInfo.isDarkerSystemColorsEnabled().catch(e => e);
140+
141+
invariant(
142+
result instanceof Error,
143+
'Expected isDarkerSystemColorsEnabled to reject',
144+
);
145+
expect(result.message).toEqual(
146+
'NativeAccessibilityManagerIOS.getCurrentDarkerSystemColorsState is not available',
147+
);
148+
});
149+
});
150+
});
151+
94152
afterEach(() => {
95153
mockNativeAccessibilityManagerDefault.getCurrentPrefersCrossFadeTransitionsState =
96154
mockGetCurrentPrefersCrossFadeTransitionsState;
155+
mockNativeAccessibilityManagerDefault.getCurrentDarkerSystemColorsState =
156+
mockGetCurrentDarkerSystemColorsState;
97157
/* $FlowFixMe[incompatible-type] */
98158
Platform.OS = originalPlatform;
99159
});

0 commit comments

Comments
 (0)