From 41fdc0d44d9b0aa0ca120babb8896bdba4d1eb53 Mon Sep 17 00:00:00 2001
From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com>
Date: Fri, 18 Aug 2023 01:45:04 -0700
Subject: [PATCH] Mobile: Fixes #8687: Hide markdown toolbar completely when
low on vertical space (#8688)
---
.eslintignore | 1 +
.gitignore | 1 +
.../NoteEditor/MarkdownToolbar/Toolbar.tsx | 31 ++++----
.../components/NoteEditor/NoteEditor.test.tsx | 72 +++++++++++++++++++
.../components/NoteEditor/NoteEditor.tsx | 29 ++++++--
packages/app-mobile/jest.setup.js | 15 ++++
6 files changed, 130 insertions(+), 19 deletions(-)
create mode 100644 packages/app-mobile/components/NoteEditor/NoteEditor.test.tsx
diff --git a/.eslintignore b/.eslintignore
index 417fd73f6..3141044ad 100644
--- a/.eslintignore
+++ b/.eslintignore
@@ -411,6 +411,7 @@ packages/app-mobile/components/NoteEditor/MarkdownToolbar/Toolbar.js
packages/app-mobile/components/NoteEditor/MarkdownToolbar/ToolbarButton.js
packages/app-mobile/components/NoteEditor/MarkdownToolbar/ToolbarOverflowRows.js
packages/app-mobile/components/NoteEditor/MarkdownToolbar/types.js
+packages/app-mobile/components/NoteEditor/NoteEditor.test.js
packages/app-mobile/components/NoteEditor/NoteEditor.js
packages/app-mobile/components/NoteEditor/SearchPanel.js
packages/app-mobile/components/NoteEditor/SelectionFormatting.js
diff --git a/.gitignore b/.gitignore
index 3c88e2abd..30f37a711 100644
--- a/.gitignore
+++ b/.gitignore
@@ -397,6 +397,7 @@ packages/app-mobile/components/NoteEditor/MarkdownToolbar/Toolbar.js
packages/app-mobile/components/NoteEditor/MarkdownToolbar/ToolbarButton.js
packages/app-mobile/components/NoteEditor/MarkdownToolbar/ToolbarOverflowRows.js
packages/app-mobile/components/NoteEditor/MarkdownToolbar/types.js
+packages/app-mobile/components/NoteEditor/NoteEditor.test.js
packages/app-mobile/components/NoteEditor/NoteEditor.js
packages/app-mobile/components/NoteEditor/SearchPanel.js
packages/app-mobile/components/NoteEditor/SelectionFormatting.js
diff --git a/packages/app-mobile/components/NoteEditor/MarkdownToolbar/Toolbar.tsx b/packages/app-mobile/components/NoteEditor/MarkdownToolbar/Toolbar.tsx
index fca9dbf90..fe70d9846 100644
--- a/packages/app-mobile/components/NoteEditor/MarkdownToolbar/Toolbar.tsx
+++ b/packages/app-mobile/components/NoteEditor/MarkdownToolbar/Toolbar.tsx
@@ -1,6 +1,6 @@
const React = require('react');
-import { ReactElement, useCallback, useState } from 'react';
+import { ReactElement, useCallback, useMemo, useState } from 'react';
import { LayoutChangeEvent, ScrollView, View, ViewStyle } from 'react-native';
import ToggleOverflowButton from './ToggleOverflowButton';
import ToolbarButton, { buttonSize } from './ToolbarButton';
@@ -18,19 +18,22 @@ const Toolbar = (props: ToolbarProps) => {
const [overflowButtonsVisible, setOverflowPopupVisible] = useState(false);
const [maxButtonsEachSide, setMaxButtonsEachSide] = useState(0);
- const allButtonSpecs = props.buttons.reduce((accumulator: ButtonSpec[], current: ButtonGroup) => {
- const newItems: ButtonSpec[] = [];
- for (const item of current.items) {
- if (item.visible ?? true) {
- newItems.push(item);
+ const allButtonSpecs = useMemo(() => {
+ const buttons = props.buttons.reduce((accumulator: ButtonSpec[], current: ButtonGroup) => {
+ const newItems: ButtonSpec[] = [];
+ for (const item of current.items) {
+ if (item.visible ?? true) {
+ newItems.push(item);
+ }
}
- }
- return accumulator.concat(...newItems);
- }, []);
+ return accumulator.concat(...newItems);
+ }, []);
- // Sort from highest priority to lowest
- allButtonSpecs.sort((a, b) => (b.priority ?? 0) - (a.priority ?? 0));
+ // Sort from highest priority to lowest
+ buttons.sort((a, b) => (b.priority ?? 0) - (a.priority ?? 0));
+ return buttons;
+ }, [props.buttons]);
const allButtonComponents: ReactElement[] = [];
let key = 0;
@@ -67,7 +70,9 @@ const Toolbar = (props: ToolbarProps) => {
);
const mainButtons: ReactElement[] = [];
- if (maxButtonsEachSide < allButtonComponents.length) {
+ if (maxButtonsEachSide >= allButtonComponents.length) {
+ mainButtons.push(...allButtonComponents);
+ } else if (maxButtonsEachSide > 0) {
// We want the menu to look something like this:
// B I (…) 🔍 ⌨
// where (…) shows/hides overflow.
@@ -77,7 +82,7 @@ const Toolbar = (props: ToolbarProps) => {
mainButtons.push(toggleOverflowButton);
mainButtons.push(...allButtonComponents.slice(-maxButtonsEachSide));
} else {
- mainButtons.push(...allButtonComponents);
+ mainButtons.push(toggleOverflowButton);
}
const styles = props.styleSheet.styles;
diff --git a/packages/app-mobile/components/NoteEditor/NoteEditor.test.tsx b/packages/app-mobile/components/NoteEditor/NoteEditor.test.tsx
new file mode 100644
index 000000000..b748725f9
--- /dev/null
+++ b/packages/app-mobile/components/NoteEditor/NoteEditor.test.tsx
@@ -0,0 +1,72 @@
+import * as React from 'react';
+
+import { describe, it, expect, beforeEach } from '@jest/globals';
+import { act, fireEvent, render, screen, waitFor } from '@testing-library/react-native';
+import '@testing-library/jest-native';
+
+import NoteEditor from './NoteEditor';
+import Setting from '@joplin/lib/models/Setting';
+import { _ } from '@joplin/lib/locale';
+import { MenuProvider } from 'react-native-popup-menu';
+import { setupDatabaseAndSynchronizer, switchClient } from '@joplin/lib/testing/test-utils';
+
+describe('NoteEditor', () => {
+ beforeEach(async () => {
+ // Required to use ExtendedWebView
+ await setupDatabaseAndSynchronizer(0);
+ await switchClient(0);
+ });
+
+ it('should hide the markdown toolbar when the window is small', async () => {
+ const wrappedNoteEditor = render(
+
+ {}}
+ onSelectionChange={()=>{}}
+ onUndoRedoDepthChange={()=>{}}
+ onAttach={()=>{}}
+ />
+
+ );
+
+ // Maps from screen height to whether the markdown toolbar should be visible.
+ const testCases: [number, boolean][] = [
+ [10, false],
+ [1000, true],
+ [100, false],
+ [80, false],
+ [600, true],
+ ];
+
+ const noteEditorRoot = await wrappedNoteEditor.findByTestId('note-editor-root');
+
+ const setRootHeight = (height: number) => {
+ act(() => {
+ // See https://stackoverflow.com/a/61774123
+ fireEvent(noteEditorRoot, 'layout', {
+ nativeEvent: {
+ layout: { height },
+ },
+ });
+ });
+ };
+
+ for (const [height, visible] of testCases) {
+ setRootHeight(height);
+
+ await waitFor(async () => {
+ const showMoreButton = await screen.queryByLabelText(_('Show more actions'));
+ if (visible) {
+ expect(showMoreButton).not.toBeNull();
+ } else {
+ expect(showMoreButton).toBeNull();
+ }
+ });
+ }
+ });
+});
diff --git a/packages/app-mobile/components/NoteEditor/NoteEditor.tsx b/packages/app-mobile/components/NoteEditor/NoteEditor.tsx
index 49a07d1a4..b9f109265 100644
--- a/packages/app-mobile/components/NoteEditor/NoteEditor.tsx
+++ b/packages/app-mobile/components/NoteEditor/NoteEditor.tsx
@@ -8,7 +8,7 @@ import ExtendedWebView from '../ExtendedWebView';
const React = require('react');
import { forwardRef, RefObject, useImperativeHandle } from 'react';
import { useEffect, useMemo, useState, useCallback, useRef } from 'react';
-import { View, ViewStyle } from 'react-native';
+import { LayoutChangeEvent, View, ViewStyle } from 'react-native';
const { editorFont } = require('../global-style');
import SelectionFormatting from './SelectionFormatting';
@@ -368,6 +368,19 @@ function NoteEditor(props: Props, ref: any) {
console.error('NoteEditor: webview error');
}, []);
+ const [hasSpaceForToolbar, setHasSpaceForToolbar] = useState(true);
+ const toolbarEnabled = props.toolbarEnabled && hasSpaceForToolbar;
+
+ const onContainerLayout = useCallback((event: LayoutChangeEvent) => {
+ const containerHeight = event.nativeEvent.layout.height;
+
+ if (containerHeight < 140) {
+ setHasSpaceForToolbar(false);
+ } else {
+ setHasSpaceForToolbar(true);
+ }
+ }, []);
+
const toolbar =
+
- {props.toolbarEnabled ? toolbar : null}
+ {toolbarEnabled ? toolbar : null}
);
}
diff --git a/packages/app-mobile/jest.setup.js b/packages/app-mobile/jest.setup.js
index 5f3d99670..152096cb3 100644
--- a/packages/app-mobile/jest.setup.js
+++ b/packages/app-mobile/jest.setup.js
@@ -33,6 +33,21 @@ document.createRange = () => {
shimInit({ nodeSqlite: sqlite3 });
+// This library has the following error when running within Jest:
+// Invariant Violation: `new NativeEventEmitter()` requires a non-null argument.
+jest.mock('react-native-device-info', () => {
+ return {
+ hasNotch: () => false,
+ };
+});
+
+// react-native-webview expects native iOS/Android code so needs to be mocked.
+jest.mock('react-native-webview', () => {
+ const { View } = require('react-native');
+ return {
+ WebView: View,
+ };
+});
// react-native-fs's CachesDirectoryPath export doesn't work in a testing environment.
// Use a temporary folder instead.