From a2071bfed29a791b089ba679a015f2c3d9df6556 Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Mon, 8 Apr 2024 04:36:40 -0700 Subject: [PATCH] Chore: Mobile: Plugin settings screen: Improve accessibility and tests (#10267) --- .eslintignore | 3 +- .gitignore | 3 +- .../plugins/PluginBox/ActionButton.tsx | 35 ++++++++++++++ .../{PluginBox.tsx => PluginBox/index.tsx} | 48 +++++++++---------- .../plugins/PluginStates.test.tsx | 21 ++++---- 5 files changed, 73 insertions(+), 37 deletions(-) create mode 100644 packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox/ActionButton.tsx rename packages/app-mobile/components/screens/ConfigScreen/plugins/{PluginBox.tsx => PluginBox/index.tsx} (81%) diff --git a/.eslintignore b/.eslintignore index e3c12f8cb..29e931cb3 100644 --- a/.eslintignore +++ b/.eslintignore @@ -577,7 +577,8 @@ packages/app-mobile/components/screens/ConfigScreen/SettingItem.js packages/app-mobile/components/screens/ConfigScreen/SettingsButton.js packages/app-mobile/components/screens/ConfigScreen/SettingsToggle.js packages/app-mobile/components/screens/ConfigScreen/configScreenStyles.js -packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox.js +packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox/ActionButton.js +packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox/index.js packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.js packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.js packages/app-mobile/components/screens/ConfigScreen/plugins/PluginToggle.js diff --git a/.gitignore b/.gitignore index 2386f2d8e..862f80aa9 100644 --- a/.gitignore +++ b/.gitignore @@ -557,7 +557,8 @@ packages/app-mobile/components/screens/ConfigScreen/SettingItem.js packages/app-mobile/components/screens/ConfigScreen/SettingsButton.js packages/app-mobile/components/screens/ConfigScreen/SettingsToggle.js packages/app-mobile/components/screens/ConfigScreen/configScreenStyles.js -packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox.js +packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox/ActionButton.js +packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox/index.js packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.js packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.js packages/app-mobile/components/screens/ConfigScreen/plugins/PluginToggle.js diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox/ActionButton.tsx b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox/ActionButton.tsx new file mode 100644 index 000000000..d7e1f53c2 --- /dev/null +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox/ActionButton.tsx @@ -0,0 +1,35 @@ +import * as React from 'react'; +import { useCallback } from 'react'; +import { ItemEvent, PluginItem } from '@joplin/lib/components/shared/config/plugins/types'; +import { Button, ButtonProps } from 'react-native-paper'; + +export type PluginCallback = (event: ItemEvent)=> void; + +interface Props extends Omit { + item: PluginItem; + onPress?: PluginCallback; + title: string; +} + +const ActionButton: React.FC = props => { + const onPress = useCallback(() => { + props.onPress?.({ item: props.item }); + }, [props.onPress, props.item]); + + // Include additional information about the button when using a screen + // reader (helps make it clear which plugin a delete/enable button). + // + // Because this is being read by a screen reader and to reduce load on + // translators, the method of joining the title and manifest name is not + // marked as translatable. + const accessibilityLabel = `${props.title} ${props.item.manifest.name}`; + return ( + + ); +}; + +export default ActionButton; diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox.tsx b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox/index.tsx similarity index 81% rename from packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox.tsx rename to packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox/index.tsx index b7bb7716f..bb8bd3de4 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox.tsx +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginBox/index.tsx @@ -1,10 +1,11 @@ import * as React from 'react'; -import { Icon, Button, Card, Chip } from 'react-native-paper'; +import { Icon, Card, Chip } from 'react-native-paper'; import { _ } from '@joplin/lib/locale'; import { View } from 'react-native'; -import { ItemEvent, PluginItem } from '@joplin/lib/components/shared/config/plugins/types'; +import { PluginItem } from '@joplin/lib/components/shared/config/plugins/types'; import shim from '@joplin/lib/shim'; import PluginService from '@joplin/lib/services/plugins/PluginService'; +import ActionButton, { PluginCallback } from './ActionButton'; export enum InstallState { NotInstalled, @@ -19,8 +20,6 @@ export enum UpdateState { HasBeenUpdated = 4, } -type PluginCallback = (event: ItemEvent)=> void; - interface Props { item: PluginItem; isCompatible: boolean; @@ -52,41 +51,42 @@ const PluginBox: React.FC = props => { }; const installButton = ( - + title={installButtonTitle()} + /> ); - const updateButtonTitle = () => { + const getUpdateButtonTitle = () => { if (props.updateState === UpdateState.Updating) return _('Updating...'); if (props.updateState === UpdateState.HasBeenUpdated) return _('Updated'); return _('Update'); }; const updateButton = ( - + title={getUpdateButtonTitle()} + /> ); + const deleteButton = ( - + title={props.item.deleted ? _('Deleted') : _('Delete')} + /> ); - const disableButton = ; - const enableButton = ; - const aboutButton = ; + const disableButton = ; + const enableButton = ; + const aboutButton = ; const renderErrorsChip = () => { if (!props.hasErrors) return null; @@ -97,7 +97,7 @@ const PluginBox: React.FC = props => { mode='outlined' onPress={() => props.onShowPluginLog({ item })} > - Error + {_('Error')} ); }; diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx index d0a8f49a9..f311c4eb0 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import RepositoryApi from '@joplin/lib/services/plugins/RepositoryApi'; import { afterAllCleanUp, afterEachCleanUp, createTempDir, mockMobilePlatform, setupDatabaseAndSynchronizer, supportDir, switchClient } from '@joplin/lib/testing/test-utils'; -import { render, screen, waitFor } from '@testing-library/react-native'; +import { render, screen } from '@testing-library/react-native'; import '@testing-library/react-native/extend-expect'; import Setting from '@joplin/lib/models/Setting'; @@ -124,18 +124,17 @@ describe('PluginStates', () => { initialPluginSettings={defaultPluginSettings} />, ); - expect(await screen.findByText('ABC Sheet Music')).not.toBeNull(); - expect(await screen.findByText('Backlinks to note')).not.toBeNull(); + expect(await screen.findByText('ABC Sheet Music')).toBeVisible(); + expect(await screen.findByText('Backlinks to note')).toBeVisible(); - const shouldBothBeUpdatable = platform === 'android'; - await waitFor(async () => { - const updateButtons = await screen.findAllByText('Update'); - expect(updateButtons).toHaveLength(shouldBothBeUpdatable ? 2 : 1); - }); + expect(await screen.findByRole('button', { name: 'Update ABC Sheet Music', disabled: false })).toBeVisible(); - const updateButtons = await screen.findAllByText('Update'); - for (const button of updateButtons) { - expect(button).not.toBeDisabled(); + // Backlinks to note should not be updatable on iOS (it's not _recommended). + const backlinksToNoteQuery = { name: 'Update Backlinks to note', disabled: false }; + if (platform === 'android') { + expect(await screen.findByRole('button', backlinksToNoteQuery)).toBeVisible(); + } else { + expect(await screen.queryByRole('button', backlinksToNoteQuery)).toBeNull(); } }); });