Merge branch 'pr/mobile/implement-toolbar-redesign'

.eslintignore

@@ -602,7 +602,7 @@ packages/app-mobile/components/EditorToolbar/EditorToolbar.js
 packages/app-mobile/components/EditorToolbar/SettingButton.js
 packages/app-mobile/components/EditorToolbar/ToolbarButton.js
 packages/app-mobile/components/EditorToolbar/ToolbarEditorDialog.js
-packages/app-mobile/components/EditorToolbar/utils/allCommandNamesFromState.js
+packages/app-mobile/components/EditorToolbar/utils/allToolbarCommandNamesFromState.js
 packages/app-mobile/components/EditorToolbar/utils/isSelected.js
 packages/app-mobile/components/EditorToolbar/utils/selectedCommandNamesFromState.js
 packages/app-mobile/components/EditorToolbar/utils/toolbarButtonsFromState.js

.gitignore

@@ -578,7 +578,7 @@ packages/app-mobile/components/EditorToolbar/EditorToolbar.js
 packages/app-mobile/components/EditorToolbar/SettingButton.js
 packages/app-mobile/components/EditorToolbar/ToolbarButton.js
 packages/app-mobile/components/EditorToolbar/ToolbarEditorDialog.js
-packages/app-mobile/components/EditorToolbar/utils/allCommandNamesFromState.js
+packages/app-mobile/components/EditorToolbar/utils/allToolbarCommandNamesFromState.js
 packages/app-mobile/components/EditorToolbar/utils/isSelected.js
 packages/app-mobile/components/EditorToolbar/utils/selectedCommandNamesFromState.js
 packages/app-mobile/components/EditorToolbar/utils/toolbarButtonsFromState.js

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

@@ -10,6 +10,8 @@ import { setupDatabase, switchClient } from '@joplin/lib/testing/test-utils';
 import createMockReduxStore from '../../utils/testing/createMockReduxStore';
 import setGlobalStore from '../../utils/testing/setGlobalStore';
 import Setting from '@joplin/lib/models/Setting';
+import CommandService, { CommandRuntime, RegisteredRuntime } from '@joplin/lib/services/CommandService';
+import allToolbarCommandNamesFromState from './utils/allToolbarCommandNamesFromState';
 
 let store: Store<AppState>;
 
@@ -28,6 +30,10 @@ const queryToolbarButton = (label: string) => {
 const openSettings = async () => {
 	const settingButton = screen.getByRole('button', { name: 'Settings' });
 	fireEvent.press(settingButton);
+
+	// Settings should be open:
+	const settingsHeader = await screen.findByRole('heading', { name: 'Manage toolbar options' });
+	expect(settingsHeader).toBeVisible();
 };
 
 interface ToggleSettingItemProps {
@@ -48,13 +54,45 @@ const toggleSettingsItem = async (props: ToggleSettingItemProps) => {
 	});
 };
 
+let mockCommands: RegisteredRuntime|null = null;
+// The toolbar expects all toolbar command runtimes to be registered before it can be
+// rendered:
+const mockCommandRuntimes = (store: Store<AppState>) => {
+	const makeMockRuntime = (commandName: string) => ({
+		declaration: { name: commandName },
+		runtime: (_props: null): CommandRuntime => ({
+			execute: jest.fn(),
+		}),
+	});
+
+	const isSeparator = (commandName: string) => commandName === '-';
+
+	const mockRuntimes = allToolbarCommandNamesFromState(
+		store.getState(),
+	).filter(
+		name => !isSeparator(name),
+	).map(makeMockRuntime);
+	return CommandService.instance().componentRegisterCommands(
+		null, mockRuntimes,
+	);
+};
+
 describe('EditorToolbar', () => {
 	beforeEach(async () => {
 		await setupDatabase(0);
 		await switchClient(0);
 
 		store = createMockReduxStore();
 		setGlobalStore(store);
+		mockCommands = mockCommandRuntimes(store);
+
+		// Start with the default set of buttons
+		Setting.setValue('editor.toolbarButtons', []);
+	});
+
+	afterEach(() => {
+		mockCommands?.deregister();
+		mockCommands = null;
 	});
 
 	it('unchecking items in settings should remove them from the toolbar', async () => {
@@ -97,4 +135,26 @@ describe('EditorToolbar', () => {
 
 		toolbar.unmount();
 	});
+
+	it('should only include the math toolbar button if math is enabled in global settings', async () => {
+		Setting.setValue('editor.toolbarButtons', ['textMath']);
+		Setting.setValue('markdown.plugin.katex', true);
+
+		const toolbar = render(<WrappedToolbar/>);
+
+		// Should initially show in the toolbar
+		expect(queryToolbarButton('Math')).toBeVisible();
+
+		// After disabled: Should not show in the toolbar
+		await waitFor(() => {
+			Setting.setValue('markdown.plugin.katex', false);
+			expect(queryToolbarButton('Math')).toBeNull();
+		});
+
+		// Should not show in settings
+		await openSettings();
+		expect(screen.queryByRole('checkbox', { name: 'Math' })).toBeNull();
+
+		toolbar.unmount();
+	});
 });

packages/app-mobile/components/EditorToolbar/ToolbarEditorDialog.tsx

@@ -10,11 +10,13 @@ import ToolbarButtonUtils, { ToolbarButtonInfo, ToolbarItem } from '@joplin/lib/
 import Icon from '../Icon';
 import { AppState } from '../../utils/types';
 import CommandService from '@joplin/lib/services/CommandService';
-import allCommandNamesFromState from './utils/allCommandNamesFromState';
+import allToolbarCommandNamesFromState from './utils/allToolbarCommandNamesFromState';
 import Setting from '@joplin/lib/models/Setting';
 import DismissibleDialog, { DialogSize } from '../DismissibleDialog';
 import selectedCommandNamesFromState from './utils/selectedCommandNamesFromState';
 import stateToWhenClauseContext from '../../services/commands/stateToWhenClauseContext';
+import { DeleteButton } from '../buttons';
+import shim from '@joplin/lib/shim';
 
 const toolbarButtonUtils = new ToolbarButtonUtils(CommandService.instance());
 
@@ -23,6 +25,7 @@ interface EditorDialogProps {
 	defaultToolbarButtonInfos: ToolbarItem[];
 	selectedCommandNames: string[];
 	allCommandNames: string[];
+	hasCustomizedLayout: boolean;
 
 	visible: boolean;
 	onDismiss: ()=> void;
@@ -45,8 +48,8 @@ const useStyle = (themeId: number) => {
 				marginTop: theme.marginTop,
 				flex: 1,
 			},
-			listItemButton: {
-
+			resetButton: {
+				marginTop: theme.marginTop,
 			},
 			listItem: {
 				flexDirection: 'row',
@@ -101,7 +104,6 @@ const ToolbarItemToggle: React.FC<ItemToggleProps> = ({
 
 	return (
 		<TouchableRipple
-			style={styles.listItemButton}
 			accessibilityRole='checkbox'
 			accessibilityState={{ checked }}
 			aria-checked={checked}
@@ -135,6 +137,25 @@ const ToolbarEditorScreen: React.FC<EditorDialogProps> = props => {
 		/>;
 	};
 
+	const onRestoreDefaultLayout = useCallback(async () => {
+		// Dismiss before showing the confirm dialog to prevent modal conflicts.
+		// On some platforms (web and possibly iOS) showing multiple modals
+		// at the same time can cause issues.
+		props.onDismiss();
+
+		const message = _('Are you sure that you want to restore the default toolbar layout?\nThis cannot be undone.');
+		if (await shim.showConfirmationDialog(message)) {
+			Setting.setValue('editor.toolbarButtons', []);
+		}
+	}, [props.onDismiss]);
+
+	const restoreButton = <DeleteButton
+		style={styles.resetButton}
+		onPress={onRestoreDefaultLayout}
+	>
+		{_('Restore default')}
+	</DeleteButton>;
+
 	return (
 		<DismissibleDialog
 			size={DialogSize.Small}
@@ -143,11 +164,12 @@ const ToolbarEditorScreen: React.FC<EditorDialogProps> = props => {
 			onDismiss={props.onDismiss}
 		>
 			<View>
-				<Text variant='headlineMedium'>{_('Manage toolbar options')}</Text>
+				<Text variant='headlineMedium' role='heading'>{_('Manage toolbar options')}</Text>
 				<Text variant='labelMedium'>{_('Check elements to display in the toolbar')}</Text>
 			</View>
 			<ScrollView style={styles.listContainer}>
 				{props.defaultToolbarButtonInfos.map((item, index) => renderItem(item, index))}
+				{props.hasCustomizedLayout ? restoreButton : null}
 			</ScrollView>
 		</DismissibleDialog>
 	);
@@ -156,13 +178,14 @@ const ToolbarEditorScreen: React.FC<EditorDialogProps> = props => {
 export default connect((state: AppState) => {
 	const whenClauseContext = stateToWhenClauseContext(state);
 
-	const allCommandNames = allCommandNamesFromState(state);
+	const allCommandNames = allToolbarCommandNamesFromState(state);
 	const selectedCommandNames = selectedCommandNamesFromState(state);
 
 	return {
 		themeId: state.settings.theme,
 		selectedCommandNames,
 		allCommandNames,
+		hasCustomizedLayout: state.settings['editor.toolbarButtons'].length > 0,
 		defaultToolbarButtonInfos: toolbarButtonUtils.commandsToToolbarButtons(allCommandNames, whenClauseContext),
 	};
 })(ToolbarEditorScreen);

packages/app-mobile/components/EditorToolbar/utils/allCommandNamesFromState.ts → packages/app-mobile/components/EditorToolbar/utils/allToolbarCommandNamesFromState.ts

@@ -30,14 +30,25 @@ const builtInCommandNames = [
 ];
 
 
-const allCommandNamesFromState = (state: AppState) => {
+const allToolbarCommandNamesFromState = (state: AppState) => {
 	const pluginCommandNames = pluginUtils.commandNamesFromViews(state.pluginService.plugins, 'editorToolbar');
 
+	let allCommandNames = builtInCommandNames;
 	if (pluginCommandNames.length > 0) {
-		return builtInCommandNames.concat(['-'], pluginCommandNames);
+		allCommandNames = allCommandNames.concat(['-'], pluginCommandNames);
 	}
 
-	return builtInCommandNames;
+	// If the user disables math markup, the "toggle math" button won't be useful.
+	// Disabling the math markup button maintains compatibility with the previous
+	// toolbar.
+	const mathEnabled = state.settings['markdown.plugin.katex'];
+	if (!mathEnabled) {
+		allCommandNames = allCommandNames.filter(
+			name => name !== EditorCommandType.ToggleMath,
+		);
+	}
+
+	return allCommandNames;
 };
 
-export default allCommandNamesFromState;
+export default allToolbarCommandNamesFromState;

packages/app-mobile/components/EditorToolbar/utils/selectedCommandNamesFromState.ts

@@ -1,6 +1,6 @@
 import { EditorCommandType } from '@joplin/editor/types';
 import { AppState } from '../../../utils/types';
-import allCommandNamesFromState from './allCommandNamesFromState';
+import allToolbarCommandNamesFromState from './allToolbarCommandNamesFromState';
 import { Platform } from 'react-native';
 
 const omitFromDefault: string[] = [
@@ -17,7 +17,7 @@ if (Platform.OS !== 'ios') {
 }
 
 const selectedCommandNamesFromState = (state: AppState) => {
-	const allCommandNames = allCommandNamesFromState(state);
+	const allCommandNames = allToolbarCommandNamesFromState(state);
 	const defaultCommandNames = allCommandNames.filter(commandName => {
 		return !omitFromDefault.includes(commandName);
 	});

packages/app-mobile/components/buttons/index.tsx

@@ -12,3 +12,4 @@ const makeTextButtonComponent = (type: ButtonType) => {
 export const PrimaryButton = makeTextButtonComponent(ButtonType.Primary);
 export const SecondaryButton = makeTextButtonComponent(ButtonType.Secondary);
 export const LinkButton = makeTextButtonComponent(ButtonType.Link);
+export const DeleteButton = makeTextButtonComponent(ButtonType.Delete);

packages/app-mobile/components/screens/Note/Note.tsx

@@ -522,7 +522,6 @@ class NoteScreenComponent extends BaseScreenComponent<ComponentProps, State> imp
 				},
 			},
 			commands,
-			true,
 		);
 	}
 

packages/lib/services/CommandService.ts

@@ -257,7 +257,7 @@ export default class CommandService extends BaseService {
 		}
 	}
 
-	public componentRegisterCommands<ComponentType>(component: ComponentType, commands: ComponentCommandSpec<ComponentType>[], allowMultiple?: boolean) {
+	public componentRegisterCommands<ComponentType>(component: ComponentType, commands: ComponentCommandSpec<ComponentType>[], allowMultiple?: boolean): RegisteredRuntime {
 		const runtimeHandles: RegisteredRuntime[] = [];
 		for (const command of commands) {
 			runtimeHandles.push(

packages/lib/services/commands/ToolbarButtonUtils.ts

@@ -57,8 +57,7 @@ export default class ToolbarButtonUtils {
 			return this.toolbarButtonCache_[commandName].info;
 		}
 
-		// The command's runtime may not be available until soon after the component is mounted:
-		const command = this.service.commandByName(commandName, { runtimeMustBeRegistered: false });
+		const command = this.service.commandByName(commandName, { runtimeMustBeRegistered: true });
 
 		const output: ToolbarButtonInfo = {
 			type: 'button',