From 18ebd164287a60373a3341e4c33a805d2bc8901e Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Sat, 29 Mar 2025 05:46:37 -0700 Subject: [PATCH] iOS: Fixes #11711: Fix Markdown toolbar partially covered by keyboard on some iOS devices (#12027) --- .eslintignore | 3 +- .gitignore | 3 +- .../components/ToggleSpaceButton.tsx | 97 ------------ packages/app-mobile/components/app-nav.tsx | 146 ++++++------------ .../components/screens/Note/Note.tsx | 17 +- .../utils/hooks/useKeyboardState.ts | 35 +++++ .../utils/hooks/useKeyboardVisible.ts | 27 ---- .../lib/models/settings/builtInMetadata.ts | 16 -- 8 files changed, 86 insertions(+), 258 deletions(-) delete mode 100644 packages/app-mobile/components/ToggleSpaceButton.tsx create mode 100644 packages/app-mobile/utils/hooks/useKeyboardState.ts delete mode 100644 packages/app-mobile/utils/hooks/useKeyboardVisible.ts diff --git a/.eslintignore b/.eslintignore index 0d741e902d..7dd8cad832 100644 --- a/.eslintignore +++ b/.eslintignore @@ -686,7 +686,6 @@ packages/app-mobile/components/SelectDateTimeDialog.js packages/app-mobile/components/SideMenu.js packages/app-mobile/components/SideMenuContentNote.js packages/app-mobile/components/TextInput.js -packages/app-mobile/components/ToggleSpaceButton.js packages/app-mobile/components/accessibility/AccessibleModalMenu.js packages/app-mobile/components/accessibility/AccessibleView.test.js packages/app-mobile/components/accessibility/AccessibleView.js @@ -859,7 +858,7 @@ packages/app-mobile/utils/fs-driver/testUtil/createFilesFromPathRecord.js packages/app-mobile/utils/fs-driver/testUtil/verifyDirectoryMatches.js packages/app-mobile/utils/getPackageInfo.js packages/app-mobile/utils/getVersionInfoText.js -packages/app-mobile/utils/hooks/useKeyboardVisible.js +packages/app-mobile/utils/hooks/useKeyboardState.js packages/app-mobile/utils/hooks/useOnLongPressProps.js packages/app-mobile/utils/hooks/useReduceMotionEnabled.js packages/app-mobile/utils/image/fileToImage.web.js diff --git a/.gitignore b/.gitignore index e67719f994..c12a756f06 100644 --- a/.gitignore +++ b/.gitignore @@ -661,7 +661,6 @@ packages/app-mobile/components/SelectDateTimeDialog.js packages/app-mobile/components/SideMenu.js packages/app-mobile/components/SideMenuContentNote.js packages/app-mobile/components/TextInput.js -packages/app-mobile/components/ToggleSpaceButton.js packages/app-mobile/components/accessibility/AccessibleModalMenu.js packages/app-mobile/components/accessibility/AccessibleView.test.js packages/app-mobile/components/accessibility/AccessibleView.js @@ -834,7 +833,7 @@ packages/app-mobile/utils/fs-driver/testUtil/createFilesFromPathRecord.js packages/app-mobile/utils/fs-driver/testUtil/verifyDirectoryMatches.js packages/app-mobile/utils/getPackageInfo.js packages/app-mobile/utils/getVersionInfoText.js -packages/app-mobile/utils/hooks/useKeyboardVisible.js +packages/app-mobile/utils/hooks/useKeyboardState.js packages/app-mobile/utils/hooks/useOnLongPressProps.js packages/app-mobile/utils/hooks/useReduceMotionEnabled.js packages/app-mobile/utils/image/fileToImage.web.js diff --git a/packages/app-mobile/components/ToggleSpaceButton.tsx b/packages/app-mobile/components/ToggleSpaceButton.tsx deleted file mode 100644 index 7ec8a66698..0000000000 --- a/packages/app-mobile/components/ToggleSpaceButton.tsx +++ /dev/null @@ -1,97 +0,0 @@ - -// On some devices, the SafeAreaView conflicts with the KeyboardAvoidingView, creating -// additional (or a lack of additional) space at the bottom of the screen. Because this -// is different on different devices, this button allows toggling additional space a the bottom -// of the screen to compensate. - -// Works around https://github.com/facebook/react-native/issues/13393 by adding additional -// space below the given component when the keyboard is visible unless a button is pressed. - -import Setting from '@joplin/lib/models/Setting'; -import { themeStyle } from '@joplin/lib/theme'; - -import * as React from 'react'; -import { ReactNode, useCallback, useState, useEffect } from 'react'; -import { Platform, useWindowDimensions, View, ViewStyle } from 'react-native'; -import IconButton from './IconButton'; -import useKeyboardVisible from '../utils/hooks/useKeyboardVisible'; - -interface Props { - children: ReactNode; - themeId: number; - style?: ViewStyle; -} - -const ToggleSpaceButton = (props: Props) => { - const [additionalSpace, setAdditionalSpace] = useState(0); - const [decreaseSpaceBtnVisible, setDecreaseSpaceBtnVisible] = useState(true); - - // Some devices need space added, others need space removed. - const additionalPositiveSpace = 14; - const additionalNegativeSpace = -14; - - // Switch from adding +14px to -14px. - const onDecreaseSpace = useCallback(() => { - setAdditionalSpace(additionalNegativeSpace); - setDecreaseSpaceBtnVisible(false); - Setting.setValue('editor.mobile.removeSpaceBelowToolbar', true); - }, [setAdditionalSpace, setDecreaseSpaceBtnVisible, additionalNegativeSpace]); - - useEffect(() => { - if (Setting.value('editor.mobile.removeSpaceBelowToolbar')) { - onDecreaseSpace(); - } - }, [onDecreaseSpace]); - - const theme = themeStyle(props.themeId); - - const decreaseSpaceButton = ( - <> - - - - ); - - const { keyboardVisible } = useKeyboardVisible(); - const windowSize = useWindowDimensions(); - const isPortrait = windowSize.height > windowSize.width; - const spaceApplicable = keyboardVisible && Platform.OS === 'ios' && isPortrait; - - const style: ViewStyle = { - marginBottom: spaceApplicable ? additionalSpace : 0, - ...props.style, - }; - - return ( - - {props.children} - { decreaseSpaceBtnVisible && spaceApplicable ? decreaseSpaceButton : null } - - ); -}; - -export default ToggleSpaceButton; diff --git a/packages/app-mobile/components/app-nav.tsx b/packages/app-mobile/components/app-nav.tsx index 9248fb7140..1a44d1eb6f 100644 --- a/packages/app-mobile/components/app-nav.tsx +++ b/packages/app-mobile/components/app-nav.tsx @@ -2,15 +2,12 @@ import * as React from 'react'; import { connect } from 'react-redux'; import NotesScreen from './screens/Notes'; import SearchScreen from './screens/SearchScreen'; -import { Component } from 'react'; -import { KeyboardAvoidingView, Keyboard, Platform, View, KeyboardEvent, Dimensions, EmitterSubscription } from 'react-native'; +import { KeyboardAvoidingView, Platform, View } from 'react-native'; import { AppState } from '../utils/types'; import { themeStyle } from './global-style'; - -interface State { - autoCompletionBarExtraHeight: number; - floatingKeyboardEnabled: boolean; -} +import { useSafeAreaInsets } from 'react-native-safe-area-context'; +import useKeyboardState from '../utils/hooks/useKeyboardState'; +import usePrevious from '@joplin/lib/hooks/usePrevious'; interface Props { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied @@ -22,107 +19,58 @@ interface Props { themeId: number; } -class AppNavComponent extends Component { - private previousRouteName_: string|null = null; - private keyboardDidShowListener: EmitterSubscription|null = null; - private keyboardDidHideListener: EmitterSubscription|null = null; - private keyboardWillChangeFrameListener: EmitterSubscription|null = null; +const AppNavComponent: React.FC = (props) => { + const keyboardState = useKeyboardState(); + const safeAreaPadding = useSafeAreaInsets(); - public constructor(props: Props) { - super(props); + if (!props.route) throw new Error('Route must not be null'); - this.previousRouteName_ = null; - this.state = { - autoCompletionBarExtraHeight: 0, // Extra padding for the auto completion bar at the top of the keyboard - floatingKeyboardEnabled: false, - }; + // Note: certain screens are kept into memory, in particular Notes and Search + // so that the scroll position is not lost when the user navigate away from them. + + const route = props.route; + let Screen = null; + let notesScreenVisible = false; + let searchScreenVisible = false; + + if (route.routeName === 'Notes') { + notesScreenVisible = true; + } else if (route.routeName === 'Search') { + searchScreenVisible = true; + } else { + Screen = props.screens[route.routeName].screen; } - public UNSAFE_componentWillMount() { - if (Platform.OS === 'ios') { - this.keyboardDidShowListener = Keyboard.addListener('keyboardDidShow', this.keyboardDidShow.bind(this)); - this.keyboardDidHideListener = Keyboard.addListener('keyboardDidHide', this.keyboardDidHide.bind(this)); - this.keyboardWillChangeFrameListener = Keyboard.addListener('keyboardWillChangeFrame', this.keyboardWillChangeFrame); - } - } + const previousRouteName = usePrevious(route.routeName, ''); - public componentWillUnmount() { - this.keyboardDidShowListener?.remove(); - this.keyboardDidHideListener?.remove(); - this.keyboardWillChangeFrameListener?.remove(); + // Keep the search screen loaded if the user is viewing a note from that search screen + // so that if the back button is pressed, the screen is still loaded. However, unload + // it if navigating away. + const searchScreenLoaded = searchScreenVisible || (previousRouteName === 'Search' && route.routeName === 'Note'); - this.keyboardDidShowListener = null; - this.keyboardDidHideListener = null; - this.keyboardWillChangeFrameListener = null; - } + const theme = themeStyle(props.themeId); - public keyboardDidShow() { - this.setState({ autoCompletionBarExtraHeight: 30 }); - } + const style = { flex: 1, backgroundColor: theme.backgroundColor }; - public keyboardDidHide() { - this.setState({ autoCompletionBarExtraHeight: 0 }); - } + // When the floating keyboard is enabled, the KeyboardAvoidingView can have a very small + // height. Don't use the KeyboardAvoidingView when the floating keyboard is enabled. + // See https://github.com/facebook/react-native/issues/29473 + const keyboardAvoidingViewEnabled = !keyboardState.isFloatingKeyboard; + const autocompletionBarPadding = Platform.OS === 'ios' && keyboardState.keyboardVisible ? safeAreaPadding.top : 0; - private keyboardWillChangeFrame = (evt: KeyboardEvent) => { - const windowWidth = Dimensions.get('window').width; - - // If the keyboard isn't as wide as the window, the floating keyboard is disabled. - // See https://github.com/facebook/react-native/issues/29473#issuecomment-696658937 - this.setState({ - floatingKeyboardEnabled: evt.endCoordinates.width < windowWidth, - }); - }; - - public render() { - if (!this.props.route) throw new Error('Route must not be null'); - - // Note: certain screens are kept into memory, in particular Notes and Search - // so that the scroll position is not lost when the user navigate away from them. - - const route = this.props.route; - let Screen = null; - let notesScreenVisible = false; - let searchScreenVisible = false; - - if (route.routeName === 'Notes') { - notesScreenVisible = true; - } else if (route.routeName === 'Search') { - searchScreenVisible = true; - } else { - Screen = this.props.screens[route.routeName].screen; - } - - // Keep the search screen loaded if the user is viewing a note from that search screen - // so that if the back button is pressed, the screen is still loaded. However, unload - // it if navigating away. - const searchScreenLoaded = searchScreenVisible || (this.previousRouteName_ === 'Search' && route.routeName === 'Note'); - - this.previousRouteName_ = route.routeName; - - const theme = themeStyle(this.props.themeId); - - const style = { flex: 1, backgroundColor: theme.backgroundColor }; - - // When the floating keyboard is enabled, the KeyboardAvoidingView can have a very small - // height. Don't use the KeyboardAvoidingView when the floating keyboard is enabled. - // See https://github.com/facebook/react-native/issues/29473 - const keyboardAvoidingViewEnabled = !this.state.floatingKeyboardEnabled; - - return ( - - - {searchScreenLoaded && } - {!notesScreenVisible && !searchScreenVisible && } - - - ); - } -} + return ( + + + {searchScreenLoaded && } + {!notesScreenVisible && !searchScreenVisible && } + + + ); +}; const AppNav = connect((state: AppState) => { return { diff --git a/packages/app-mobile/components/screens/Note/Note.tsx b/packages/app-mobile/components/screens/Note/Note.tsx index b80906b353..89f0040d86 100644 --- a/packages/app-mobile/components/screens/Note/Note.tsx +++ b/packages/app-mobile/components/screens/Note/Note.tsx @@ -61,7 +61,6 @@ import { DialogContext, DialogControl } from '../../DialogManager'; import { CommandRuntimeProps, EditorMode, PickerResponse } from './types'; import commands from './commands'; import { AttachFileAction, AttachFileOptions } from './commands/attachFile'; -import ToggleSpaceButton from '../../ToggleSpaceButton'; import PluginService from '@joplin/lib/services/plugins/PluginService'; import PluginUserWebView from '../../plugins/dialogs/PluginUserWebView'; import getShownPluginEditorView from '@joplin/lib/services/plugins/utils/getShownPluginEditorView'; @@ -1675,19 +1674,6 @@ class NoteScreenComponent extends BaseScreenComponent imp return result; }; - const renderWrappedContent = () => { - const content = <> - {bodyComponent} - {renderVoiceTypingDialogs()} - ; - - return this.state.mode === 'edit' ? ( - - {content} - - ) : content; - }; - const { editorPlugin: activeEditorPlugin } = getActivePluginEditorView(this.props.plugins); return ( @@ -1709,7 +1695,8 @@ class NoteScreenComponent extends BaseScreenComponent imp title={getDisplayParentTitle(this.state.note, this.state.folder)} /> {titleComp} - {renderWrappedContent()} + {bodyComponent} + {renderVoiceTypingDialogs()} {renderActionButton()} diff --git a/packages/app-mobile/utils/hooks/useKeyboardState.ts b/packages/app-mobile/utils/hooks/useKeyboardState.ts new file mode 100644 index 0000000000..a731eb0d63 --- /dev/null +++ b/packages/app-mobile/utils/hooks/useKeyboardState.ts @@ -0,0 +1,35 @@ +import { useEffect, useMemo, useState } from 'react'; +import { Dimensions, Keyboard } from 'react-native'; + +const useKeyboardState = () => { + const [keyboardVisible, setKeyboardVisible] = useState(false); + const [hasSoftwareKeyboard, setHasSoftwareKeyboard] = useState(false); + const [isFloatingKeyboard, setIsFloatingKeyboard] = useState(false); + useEffect(() => { + const showListener = Keyboard.addListener('keyboardDidShow', () => { + setKeyboardVisible(true); + setHasSoftwareKeyboard(true); + }); + const hideListener = Keyboard.addListener('keyboardDidHide', () => { + setKeyboardVisible(false); + }); + const floatingListener = Keyboard.addListener('keyboardWillChangeFrame', (evt) => { + const windowWidth = Dimensions.get('window').width; + // If the keyboard isn't as wide as the window, the floating keyboard is disabled. + // See https://github.com/facebook/react-native/issues/29473#issuecomment-696658937 + setIsFloatingKeyboard(evt.endCoordinates.width < windowWidth); + }); + + return (() => { + showListener.remove(); + hideListener.remove(); + floatingListener.remove(); + }); + }); + + return useMemo(() => { + return { keyboardVisible, hasSoftwareKeyboard, isFloatingKeyboard }; + }, [keyboardVisible, hasSoftwareKeyboard, isFloatingKeyboard]); +}; + +export default useKeyboardState; diff --git a/packages/app-mobile/utils/hooks/useKeyboardVisible.ts b/packages/app-mobile/utils/hooks/useKeyboardVisible.ts deleted file mode 100644 index a6aa8499cb..0000000000 --- a/packages/app-mobile/utils/hooks/useKeyboardVisible.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { useEffect, useMemo, useState } from 'react'; -import { Keyboard } from 'react-native'; - -const useKeyboardVisible = () => { - const [keyboardVisible, setKeyboardVisible] = useState(false); - const [hasSoftwareKeyboard, setHasSoftwareKeyboard] = useState(false); - useEffect(() => { - const showListener = Keyboard.addListener('keyboardDidShow', () => { - setKeyboardVisible(true); - setHasSoftwareKeyboard(true); - }); - const hideListener = Keyboard.addListener('keyboardDidHide', () => { - setKeyboardVisible(false); - }); - - return (() => { - showListener.remove(); - hideListener.remove(); - }); - }); - - return useMemo(() => { - return { keyboardVisible, hasSoftwareKeyboard }; - }, [keyboardVisible, hasSoftwareKeyboard]); -}; - -export default useKeyboardVisible; diff --git a/packages/lib/models/settings/builtInMetadata.ts b/packages/lib/models/settings/builtInMetadata.ts index f8ad72c665..eb42dd5155 100644 --- a/packages/lib/models/settings/builtInMetadata.ts +++ b/packages/lib/models/settings/builtInMetadata.ts @@ -839,22 +839,6 @@ const builtInMetadata = (Setting: typeof SettingType) => { isGlobal: true, }, - // Works around a bug in which additional space is visible beneath the toolbar on some devices. - // See https://github.com/laurent22/joplin/pull/6823 - 'editor.mobile.removeSpaceBelowToolbar': { - value: false, - type: SettingItemType.Bool, - section: 'note', - public: true, - appTypes: [AppType.Mobile], - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - show: (settings: any) => settings['editor.mobile.removeSpaceBelowToolbar'], - label: () => 'Remove extra space below the markdown toolbar', - description: () => 'Works around bug on some devices where the markdown toolbar does not touch the bottom of the screen.', - storage: SettingStorage.File, - isGlobal: true, - }, - newTodoFocus: { value: 'title', type: SettingItemType.String,