Skip to content

Commit 5c9a571

Browse files
chiciometa-codesync[bot]
authored andcommitted
Fix AccessibilityInfo.isHighTextContrastEnabled unresolved promise (#56066)
Summary: This PR fixes a bug in the `AccessibilityInfo` component. This is a follow up of the work I started with the PRs I opened in the last weeks #55920 #56019. `AccessibilityInfo` exposes the method `isHighTextContrastEnabled`. In the else 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 (in the previous PRs the same bug affected the Android branch). This means that if a user calls this method eg. on iOS without any `Platform.OS === 'android'` guard, it will result in an unresolved promise. I fixed the bug in the same way as in the other PRs: by aligning the implementation of `isHighTextContrastEnabled` to the one of other methods. I also added the tests to avoid regression, and additionally verify that the Android method is doing what we are expecting (in both cases for when `NativeAccessibilityInfoAndroid.isHighTextContrastEnabled` 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 #55378). PS I will open one (or more if I see it is too big) PR to add the missing tests for the other `AccessibilityInfo methods still not tested. ## Changelog: [GENERAL] [FIXED] - Fix AccessibilityInfo.isHighTextContrastEnabled unresolved (never ending) promise Pull Request resolved: #56066 Test Plan: This fix (as for the other PRs), was developed using TDD. I first added a failing test to reproduce that the iOS branch of the `isHighTextContrastEnabled` 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 Android counterpart. Reviewed By: javache Differential Revision: D96293309 Pulled By: vzaidman fbshipit-source-id: 98c338039bf83bd418b563f64da80c471f70907f
1 parent c625015 commit 5c9a571

2 files changed

Lines changed: 69 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
@@ -213,8 +213,8 @@ const AccessibilityInfo = {
213213
* The result is `true` when high text contrast is enabled and `false` otherwise.
214214
*/
215215
isHighTextContrastEnabled(): Promise<boolean> {
216-
return new Promise((resolve, reject) => {
217-
if (Platform.OS === 'android') {
216+
if (Platform.OS === 'android') {
217+
return new Promise((resolve, reject) => {
218218
if (NativeAccessibilityInfoAndroid?.isHighTextContrastEnabled != null) {
219219
NativeAccessibilityInfoAndroid.isHighTextContrastEnabled(resolve);
220220
} else {
@@ -224,10 +224,10 @@ const AccessibilityInfo = {
224224
),
225225
);
226226
}
227-
} else {
228-
return Promise.resolve(false);
229-
}
230-
});
227+
});
228+
} else {
229+
return Promise.resolve(false);
230+
}
231231
},
232232

233233
/**

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,26 @@ const mockNativeAccessibilityManagerDefault: {
3737
getCurrentDarkerSystemColorsState: mockGetCurrentDarkerSystemColorsState,
3838
};
3939

40+
const mockIsHighTextContrastEnabled = jest.fn(onSuccess => onSuccess(true));
41+
const mockNativeAccessibilityInfo: {
42+
isHighTextContrastEnabled: JestMockFn<
43+
[onSuccess: (isHighTextContrastEnabled: boolean) => void],
44+
void,
45+
> | null,
46+
} = {
47+
isHighTextContrastEnabled: mockIsHighTextContrastEnabled,
48+
};
49+
4050
jest.mock('../NativeAccessibilityManager', () => ({
4151
__esModule: true,
4252
default: mockNativeAccessibilityManagerDefault,
4353
}));
4454

55+
jest.mock('../NativeAccessibilityInfo', () => ({
56+
__esModule: true,
57+
default: mockNativeAccessibilityInfo,
58+
}));
59+
4560
const Platform = require('../../../Utilities/Platform').default;
4661
const AccessibilityInfo = require('../AccessibilityInfo').default;
4762
const invariant = require('invariant');
@@ -53,6 +68,7 @@ describe('AccessibilityInfo', () => {
5368
originalPlatform = Platform.OS;
5469
mockGetCurrentPrefersCrossFadeTransitionsState.mockClear();
5570
mockGetCurrentDarkerSystemColorsState.mockClear();
71+
mockIsHighTextContrastEnabled.mockClear();
5672
});
5773

5874
describe('prefersCrossFadeTransitions', () => {
@@ -149,11 +165,58 @@ describe('AccessibilityInfo', () => {
149165
});
150166
});
151167

168+
describe('isHighTextContrastEnabled', () => {
169+
describe('Android', () => {
170+
it('should call isHighTextContrastEnabled if available', async () => {
171+
/* $FlowFixMe[incompatible-type] */
172+
Platform.OS = 'android';
173+
174+
const isHighTextContrastEnabled =
175+
await AccessibilityInfo.isHighTextContrastEnabled();
176+
177+
expect(mockIsHighTextContrastEnabled).toHaveBeenCalled();
178+
expect(isHighTextContrastEnabled).toBe(true);
179+
});
180+
181+
it('should throw error if isHighTextContrastEnabled is not available', async () => {
182+
/* $FlowFixMe[incompatible-type] */
183+
Platform.OS = 'android';
184+
185+
mockNativeAccessibilityInfo.isHighTextContrastEnabled = null;
186+
187+
const result: mixed =
188+
await AccessibilityInfo.isHighTextContrastEnabled().catch(e => e);
189+
190+
invariant(
191+
result instanceof Error,
192+
'Expected isHighTextContrastEnabled to reject',
193+
);
194+
expect(result.message).toEqual(
195+
'NativeAccessibilityInfoAndroid.isHighTextContrastEnabled is not available',
196+
);
197+
});
198+
});
199+
200+
describe('iOS', () => {
201+
it('should return false', async () => {
202+
/* $FlowFixMe[incompatible-type] */
203+
Platform.OS = 'ios';
204+
205+
const isHighTextContrastEnabled =
206+
await AccessibilityInfo.isHighTextContrastEnabled();
207+
208+
expect(isHighTextContrastEnabled).toBe(false);
209+
});
210+
});
211+
});
212+
152213
afterEach(() => {
153214
mockNativeAccessibilityManagerDefault.getCurrentPrefersCrossFadeTransitionsState =
154215
mockGetCurrentPrefersCrossFadeTransitionsState;
155216
mockNativeAccessibilityManagerDefault.getCurrentDarkerSystemColorsState =
156217
mockGetCurrentDarkerSystemColorsState;
218+
mockNativeAccessibilityInfo.isHighTextContrastEnabled =
219+
mockIsHighTextContrastEnabled;
157220
/* $FlowFixMe[incompatible-type] */
158221
Platform.OS = originalPlatform;
159222
});

0 commit comments

Comments
 (0)