Mobile: Fixes #8707: Fix not all dropdown items focusable with VoiceOver (#8714)

.eslintignore

@@ -402,6 +402,7 @@ packages/app-mobile/components/ActionButton.js
 packages/app-mobile/components/BackButtonDialogBox.js
 packages/app-mobile/components/CameraView.js
 packages/app-mobile/components/CustomButton.js
+packages/app-mobile/components/Dropdown.test.js
 packages/app-mobile/components/Dropdown.js
 packages/app-mobile/components/ExtendedWebView.js
 packages/app-mobile/components/FolderPicker.js

.gitignore

@@ -388,6 +388,7 @@ packages/app-mobile/components/ActionButton.js
 packages/app-mobile/components/BackButtonDialogBox.js
 packages/app-mobile/components/CameraView.js
 packages/app-mobile/components/CustomButton.js
+packages/app-mobile/components/Dropdown.test.js
 packages/app-mobile/components/Dropdown.js
 packages/app-mobile/components/ExtendedWebView.js
 packages/app-mobile/components/FolderPicker.js

packages/app-mobile/components/Dropdown.test.tsx

@@ -0,0 +1,56 @@
+import * as React from 'react';
+
+import { describe, it, expect, jest } from '@jest/globals';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react-native';
+import '@testing-library/jest-native';
+
+import Dropdown, { DropdownListItem } from './Dropdown';
+
+describe('Dropdown', () => {
+	it('should open the dropdown on click', async () => {
+		const items: DropdownListItem[] = [];
+		for (let i = 0; i < 400; i++) {
+			items.push({ label: `Item ${i}`, value: `${i}` });
+		}
+
+		const onValueChange = jest.fn();
+
+		render(
+			<Dropdown
+				items={items}
+				selectedValue={'1'}
+				onValueChange={onValueChange}
+			/>,
+		);
+
+		// Should initially not show any other items
+		expect(screen.queryByText('Item 3')).toBeNull();
+		expect(screen.queryByText('Item 4')).toBeNull();
+
+		const openButton = screen.getByText('Item 1');
+		fireEvent.press(openButton);
+
+		// Other items should now be shown
+		await waitFor(() => {
+			expect(screen.getByText('Item 3')).not.toBeNull();
+			expect(screen.getByText('Item 4')).not.toBeNull();
+		});
+
+		// Pressing one of these items should hide the dropdown
+		fireEvent.press(screen.getByText('Item 4'));
+
+		// We haven't changed the selectedValue, so Item 301 should no longer be visible
+		await waitFor(() => {
+			expect(screen.queryByText('Item 4')).toBeNull();
+		});
+
+		expect(onValueChange).toHaveBeenLastCalledWith('4');
+
+		// The dropdown should still be clickable
+		fireEvent.press(screen.getByText('Item 1'));
+
+		await waitFor(() => {
+			expect(screen.queryByText('Item 2')).not.toBeNull();
+		});
+	});
+});

packages/app-mobile/components/Dropdown.tsx

@@ -1,8 +1,7 @@
 const React = require('react');
-import { TouchableOpacity, TouchableWithoutFeedback, Dimensions, Text, Modal, View, LayoutRectangle, ViewStyle, TextStyle } from 'react-native';
+import { TouchableOpacity, TouchableWithoutFeedback, Dimensions, Text, Modal, View, LayoutRectangle, ViewStyle, TextStyle, FlatList } from 'react-native';
 import { Component } from 'react';
 import { _ } from '@joplin/lib/locale';
-const { ItemList } = require('./ItemList.js');
 
 type ValueType = string;
 export interface DropdownListItem {
@@ -122,7 +121,7 @@ class Dropdown extends Component<DropdownProps, DropdownState> {
 			this.setState({ listVisible: false });
 		};
 
-		const itemRenderer = (item: DropdownListItem) => {
+		const itemRenderer = ({ item }: { item: DropdownListItem }) => {
 			const key = item.value ? item.value.toString() : '__null'; // The top item ("Move item to notebook...") has a null value.
 			return (
 				<TouchableOpacity
@@ -194,11 +193,15 @@ class Dropdown extends Component<DropdownProps, DropdownState> {
 					<View
 						accessibilityRole='menu'
 						style={wrapperStyle}>
-						<ItemList
+						<FlatList
 							style={itemListStyle}
-							items={this.props.items}
+							data={this.props.items}
-							itemHeight={itemHeight}
+							renderItem={itemRenderer}
-							itemRenderer={itemRenderer}
+							getItemLayout={(_data, index) => ({
+								length: itemHeight,
+								offset: itemHeight * index,
+								index,
+							})}
 						/>
 					</View>
 

packages/app-mobile/components/ItemList.js

@@ -1,110 +0,0 @@
-const React = require('react');
-const { View, ScrollView } = require('react-native');
-
-class ItemList extends React.Component {
-	constructor() {
-		super();
-
-		this.scrollTop_ = 0;
-	}
-
-	itemCount(props = null) {
-		if (props === null) props = this.props;
-		return this.props.items ? this.props.items.length : this.props.itemComponents.length;
-	}
-
-	updateStateItemIndexes(props = null, height = null) {
-		if (props === null) props = this.props;
-
-		if (height === null) {
-			if (!this.state) return;
-			height = this.state.height;
-		}
-
-		const topItemIndex = Math.max(0, Math.floor(this.scrollTop_ / props.itemHeight));
-		const visibleItemCount = Math.ceil(height / props.itemHeight);
-
-		let bottomItemIndex = topItemIndex + visibleItemCount - 1;
-		if (bottomItemIndex >= this.itemCount(props)) bottomItemIndex = this.itemCount(props) - 1;
-
-		this.setState({
-			topItemIndex: topItemIndex,
-			bottomItemIndex: bottomItemIndex,
-		});
-	}
-
-	UNSAFE_componentWillMount() {
-		this.setState({
-			topItemIndex: 0,
-			bottomItemIndex: 0,
-			height: 0,
-			itemHeight: this.props.itemHeight ? this.props.itemHeight : 0,
-		});
-
-		this.updateStateItemIndexes();
-	}
-
-	UNSAFE_componentWillReceiveProps(newProps) {
-		if (newProps.itemHeight) {
-			this.setState({
-				itemHeight: newProps.itemHeight,
-			});
-		}
-
-		this.updateStateItemIndexes(newProps);
-	}
-
-	onScroll(event) {
-		this.scrollTop_ = Math.floor(event.nativeEvent.contentOffset.y);
-		this.updateStateItemIndexes();
-	}
-
-	onLayout(event) {
-		this.setState({ height: event.nativeEvent.layout.height });
-		this.updateStateItemIndexes(null, event.nativeEvent.layout.height);
-	}
-
-	render() {
-		const style = this.props.style ? this.props.style : {};
-
-		// if (!this.props.itemHeight) throw new Error('itemHeight is required');
-
-		let itemComps = [];
-
-		if (this.props.items) {
-			const items = this.props.items;
-
-			const blankItem = function(key, height) {
-				return <View key={key} style={{ height: height }}></View>;
-			};
-
-			itemComps = [blankItem('top', this.state.topItemIndex * this.props.itemHeight)];
-
-			for (let i = this.state.topItemIndex; i <= this.state.bottomItemIndex; i++) {
-				const itemComp = this.props.itemRenderer(items[i]);
-				itemComps.push(itemComp);
-			}
-
-			itemComps.push(blankItem('bottom', (items.length - this.state.bottomItemIndex - 1) * this.props.itemHeight));
-		} else {
-			itemComps = this.props.itemComponents;
-		}
-
-		return (
-			<ScrollView
-				scrollEventThrottle={500}
-				onLayout={event => {
-					this.onLayout(event);
-				}}
-				style={style}
-				onScroll={event => {
-					this.onScroll(event);
-				}}
-			>
-				{itemComps}
-			</ScrollView>
-		);
-	}
-}
-
-module.exports = { ItemList };