Skip to content

Commit

Permalink
fix(mobile): turn native textview into uncontrolled component (#1176)
Browse files Browse the repository at this point in the history
Switching from non-plain type to plain type renders initial state.text, which wasn't being properly updated on all changes.
  • Loading branch information
moughxyz committed Jun 28, 2022
1 parent b421415 commit d198625
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 52 deletions.
Binary file not shown.
Binary file not shown.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"@types/hoist-non-react-statics/@types/react": "17.0.2"
},
"dependencies": {
"@standardnotes/snjs": "^2.118.1"
"@standardnotes/snjs": "^2.118.2"
},
"devDependencies": {
"@commitlint/cli": "^17.0.2",
Expand Down
4 changes: 2 additions & 2 deletions packages/mobile/ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ PODS:
- React-Core
- RNSVG (12.3.0):
- React-Core
- RNVectorIcons (9.1.0):
- RNVectorIcons (9.2.0):
- React-Core
- RNZipArchive (6.0.8):
- React-Core
Expand Down Expand Up @@ -747,7 +747,7 @@ SPEC CHECKSUMS:
RNShare: 4406f61af043027b695c3a0b8f39e2c2bdacde12
RNStoreReview: e05edbbf426563070524cec384ac0b8df69be801
RNSVG: 302bfc9905bd8122f08966dc2ce2d07b7b52b9f8
RNVectorIcons: 7923e585eaeb139b9f4531d25a125a1500162a0b
RNVectorIcons: fcc2f6cb32f5735b586e66d14103a74ce6ad61f8
RNZipArchive: 3f89b114cfeb89dac027fc5c7afd7e58d3dc4ebf
sn-textview: aaeeb41791e7d1784072adfb6b610db9b764acda
SNReactNative: b5e9e529c175c13f3a618e27c76cf3071213d5e1
Expand Down
4 changes: 2 additions & 2 deletions packages/mobile/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@
"@standardnotes/filepicker": "^1.16.22",
"@standardnotes/icons": "workspace:*",
"@standardnotes/react-native-aes": "^1.4.3",
"@standardnotes/react-native-textview": "1.0.2",
"@standardnotes/react-native-textview": "1.1.0",
"@standardnotes/react-native-utils": "1.0.1",
"@standardnotes/sncrypto-common": "1.9.0",
"@standardnotes/snjs": "^2.118.1",
"@standardnotes/snjs": "^2.118.2",
"@standardnotes/stylekit": "5.29.3",
"@types/styled-components-react-native": "5.1.3",
"js-base64": "^3.7.2",
Expand Down
72 changes: 39 additions & 33 deletions packages/mobile/src/Screens/Compose/Compose.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { AppStateEventType } from '@Lib/ApplicationState'
import { ComponentLoadingError, ComponentManager } from '@Lib/ComponentManager'
import { isNullOrUndefined } from '@Lib/Utils'
import { ApplicationContext, SafeApplicationContext } from '@Root/ApplicationContext'
import { AppStackNavigationProp } from '@Root/AppStack'
import { SCREEN_COMPOSE } from '@Root/Screens/screens'
Expand Down Expand Up @@ -47,7 +46,6 @@ const SAVE_TIMEOUT_NO_DEBOUNCE = 100

type State = {
title: string
text: string
saveError: boolean
webViewError?: ComponentLoadingError
webViewErrorDesc?: string
Expand All @@ -67,7 +65,7 @@ const EditingIsDisabledText = 'This note has editing disabled. Please enable edi
export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRenderingDirectly, State> {
static override contextType = ApplicationContext
override context: React.ContextType<typeof ApplicationContext>
editor: NoteViewController
controller: NoteViewController
editorViewRef: React.RefObject<SNTextView> = createRef()
saveTimeout: ReturnType<typeof setTimeout> | undefined
alreadySaved = false
Expand All @@ -93,11 +91,10 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
throw 'Unable to to find note controller'
}

this.editor = editor
this.controller = editor

this.state = {
title: this.editor.item.title,
text: this.editor.item.text,
title: this.controller.item.title,
componentViewer: undefined,
saveError: false,
webViewError: undefined,
Expand All @@ -107,12 +104,13 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
}

override componentDidMount() {
this.removeNoteInnerValueObserver = this.editor.addNoteInnerValueChangeObserver((note, source) => {
this.removeNoteInnerValueObserver = this.controller.addNoteInnerValueChangeObserver((note, source) => {
if (isPayloadSourceRetrieved(source)) {
this.setState({
title: note.title,
text: note.text,
})

this.editorViewRef.current?.setText(note.text)
}

const isTemplateNoteInsertedToBeInteractableWithEditor = source === PayloadEmitSource.LocalInserted && note.dirty
Expand Down Expand Up @@ -142,17 +140,21 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
return
}

if (!this.note) {
if (!this.controllerNote) {
return
}

void this.reloadComponentEditorState()
})

this.removeAppEventObserver = this.context?.addEventObserver(async (eventName) => {
if (!this.controller || this.controller.dealloced) {
return
}

if (eventName === ApplicationEvent.CompletedFullSync) {
/** if we're still dirty, don't change status, a sync is likely upcoming. */
if (!this.note.dirty && this.state.saveError) {
if (!this.controllerNote.dirty && this.state.saveError) {
this.showAllChangesSavedStatus()
}
} else if (eventName === ApplicationEvent.FailedSync) {
Expand All @@ -161,7 +163,7 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
* Otherwise, it means the originating sync came from somewhere else
* and we don't want to display an error here.
*/
if (this.note.dirty) {
if (this.controllerNote.dirty) {
this.showErrorStatus('Sync Unavailable (changes saved offline)')
}
} else if (eventName === ApplicationEvent.LocalDatabaseWriteError) {
Expand All @@ -178,7 +180,7 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
}
})

if (this.editor.isTemplateNote && Platform.OS === 'ios') {
if (this.controller.isTemplateNote && Platform.OS === 'ios') {
setTimeout(() => {
this.editorViewRef?.current?.focus()
}, 0)
Expand Down Expand Up @@ -221,10 +223,10 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
* on a deleted note.
*/
get noteLocked() {
if (!this.note) {
if (!this.controllerNote) {
return false
}
return this.note.locked
return this.controllerNote.locked
}

setStatus = (status: string, color?: string, wait = true) => {
Expand Down Expand Up @@ -259,8 +261,8 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
this.setStatus(message)
}

get note() {
return this.editor.item
get controllerNote() {
return this.controller.item
}

dismissKeyboard = () => {
Expand All @@ -273,10 +275,11 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
}

async associateComponentWithCurrentNote(component: SNComponent) {
const note = this.note
const note = this.controllerNote
if (!note) {
return
}

return this.context?.mutator.changeItem(component, (m: ItemMutator) => {
const mutator = m as ComponentMutator
mutator.removeDisassociatedItemId(note.uuid)
Expand All @@ -291,11 +294,11 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
webViewError: undefined,
})

const associatedEditor = this.componentManager.editorForNote(this.note)
const associatedEditor = this.componentManager.editorForNote(this.controllerNote)

/** Editors cannot interact with template notes so the note must be inserted */
if (associatedEditor && this.editor.isTemplateNote) {
await this.editor.insertTemplatedNote()
if (associatedEditor && this.controller.isTemplateNote) {
await this.controller.insertTemplatedNote()
void this.associateComponentWithCurrentNote(associatedEditor)
}

Expand All @@ -317,7 +320,7 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend

loadComponentViewer(component: SNComponent) {
this.setState({
componentViewer: this.componentManager.createComponentViewer(component, this.note.uuid),
componentViewer: this.componentManager.createComponentViewer(component, this.controllerNote.uuid),
})
}

Expand All @@ -332,26 +335,26 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
webViewError: undefined,
})

const associatedEditor = this.componentManager.editorForNote(this.note)
const associatedEditor = this.componentManager.editorForNote(this.controllerNote)
if (associatedEditor) {
this.loadComponentViewer(associatedEditor)
}
}

saveNote = async (params: { newTitle?: string; newText?: string }) => {
if (this.editor.isTemplateNote) {
await this.editor.insertTemplatedNote()
if (this.controller.isTemplateNote) {
await this.controller.insertTemplatedNote()
}

if (!this.context?.items.findItem(this.note.uuid)) {
if (!this.context?.items.findItem(this.controllerNote.uuid)) {
void this.context?.alertService.alert('Attempting to save this note has failed. The note cannot be found.')
return
}

const { newTitle, newText } = params

await this.context.mutator.changeItem(
this.note,
this.controllerNote,
(mutator) => {
const noteMutator = mutator as NoteMutator

Expand Down Expand Up @@ -384,7 +387,7 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
}

onTitleChange = (newTitle: string) => {
if (this.note.locked) {
if (this.controllerNote.locked) {
void this.context?.alertService?.alert(EditingIsDisabledText)
return
}
Expand All @@ -398,7 +401,7 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
}

onContentChange = (text: string) => {
if (this.note.locked) {
if (this.controllerNote.locked) {
void this.context?.alertService?.alert(EditingIsDisabledText)
return
}
Expand Down Expand Up @@ -478,7 +481,10 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend

override render() {
const shouldDisplayEditor =
this.state.componentViewer && Boolean(this.note) && !this.note.prefersPlainEditor && !this.state.webViewError
this.state.componentViewer &&
Boolean(this.controllerNote) &&
!this.controllerNote.prefersPlainEditor &&
!this.state.webViewError

return (
<Container>
Expand Down Expand Up @@ -543,13 +549,13 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
/>
)}

{!shouldDisplayEditor && !isNullOrUndefined(this.note) && Platform.OS === 'android' && (
{!shouldDisplayEditor && this.controllerNote != undefined && Platform.OS === 'android' && (
<TextContainer>
<StyledTextView
testID="noteContentField"
ref={this.editorViewRef}
autoFocus={false}
value={this.state.text}
defaultValue={this.controllerNote.text}
selectionColor={lighten(theme.stylekitInfoColor, 0.35)}
handlesColor={theme.stylekitInfoColor}
onChangeText={this.onContentChange}
Expand All @@ -559,13 +565,13 @@ export class Compose extends React.Component<PropsWhenNavigating | PropsWhenRend
)}
{/* Empty wrapping view fixes native textview crashing */}
{!shouldDisplayEditor && Platform.OS === 'ios' && (
<View key={this.note.uuid}>
<View key={this.controllerNote.uuid}>
<StyledTextView
testID="noteContentField"
ref={this.editorViewRef}
autoFocus={false}
multiline
value={this.state.text}
defaultValue={this.controllerNote.text}
keyboardDismissMode={'interactive'}
keyboardAppearance={themeService?.keyboardColorForActiveTheme()}
selectionColor={lighten(theme.stylekitInfoColor)}
Expand Down
2 changes: 1 addition & 1 deletion packages/mobile/src/Screens/Notes/NoteList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ export const NoteList = (props: Props) => {
<FlatList
ref={noteListRef}
style={styles.list}
keyExtractor={(item) => item?.uuid}
keyExtractor={(item) => item?.uuid || String(new Date().getTime())}
contentContainerStyle={[{ paddingBottom: insets.bottom }, props.notes.length > 0 ? {} : { height: '100%' }]}
initialNumToRender={6}
windowSize={6}
Expand Down
2 changes: 1 addition & 1 deletion packages/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
"@standardnotes/icons": "workspace:*",
"@standardnotes/services": "^1.13.22",
"@standardnotes/sncrypto-web": "1.10.1",
"@standardnotes/snjs": "^2.118.1",
"@standardnotes/snjs": "^2.118.2",
"@standardnotes/styles": "workspace:*",
"@standardnotes/toast": "workspace:*",
"@zip.js/zip.js": "^2.4.10",
Expand Down
24 changes: 12 additions & 12 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4962,7 +4962,7 @@ __metadata:
"@lerna-lite/list": ^1.5.1
"@lerna-lite/run": ^1.5.1
"@standardnotes/config": ^2.4.3
"@standardnotes/snjs": ^2.118.1
"@standardnotes/snjs": ^2.118.2
"@typescript-eslint/eslint-plugin": ^5.20.0
"@typescript-eslint/parser": ^5.20.0
changelog-parser: ^2.8.1
Expand Down Expand Up @@ -5615,10 +5615,10 @@ __metadata:
"@standardnotes/filepicker": ^1.16.22
"@standardnotes/icons": "workspace:*"
"@standardnotes/react-native-aes": ^1.4.3
"@standardnotes/react-native-textview": 1.0.2
"@standardnotes/react-native-textview": 1.1.0
"@standardnotes/react-native-utils": 1.0.1
"@standardnotes/sncrypto-common": 1.9.0
"@standardnotes/snjs": ^2.118.1
"@standardnotes/snjs": ^2.118.2
"@standardnotes/stylekit": 5.29.3
"@types/detox": ^18.1.0
"@types/faker": ^6.6.9
Expand Down Expand Up @@ -5710,13 +5710,13 @@ __metadata:
languageName: node
linkType: hard

"@standardnotes/react-native-textview@npm:1.0.2":
version: 1.0.2
resolution: "@standardnotes/react-native-textview@npm:1.0.2"
"@standardnotes/react-native-textview@npm:1.1.0":
version: 1.1.0
resolution: "@standardnotes/react-native-textview@npm:1.1.0"
peerDependencies:
react: ^16.11.0
react-native: ">=0.60.0-rc.0 <1.0.x"
checksum: 8345c2ed957bfc8fc184841ee1737243376d11c3b7b990795f99c6534d178ed9f5b48fa70063e27d05f19e83176d9b8a2e06477f707396083eaa9bf8c55c14ca
checksum: 150992e82be4a1cd02bf8fa3c094b203cc6c6b5062d220d984239731efc28a064030f2162946b85a81151672e5d22e4b215a62d1bb9abd156f75605f3d6f1f75
languageName: node
linkType: hard

Expand Down Expand Up @@ -5864,9 +5864,9 @@ __metadata:
languageName: node
linkType: hard

"@standardnotes/snjs@npm:^2.118.1":
version: 2.118.1
resolution: "@standardnotes/snjs@npm:2.118.1"
"@standardnotes/snjs@npm:^2.118.2":
version: 2.118.2
resolution: "@standardnotes/snjs@npm:2.118.2"
dependencies:
"@standardnotes/api": ^1.1.18
"@standardnotes/auth": ^3.19.4
Expand All @@ -5882,7 +5882,7 @@ __metadata:
"@standardnotes/sncrypto-common": ^1.9.0
"@standardnotes/utils": ^1.6.12
semver: ^7.3.7
checksum: fe3da9c8061994a16e2e9397881b7ace46c26740d1c0573bd97e0bd1fb8675939eb04df27b95d261968ad09e1c5ed9822ae548ab0fb7a5a01dce9f7fbc7d0380
checksum: 65ccedcced28af1d9fdc9a9294e72b138f1bc23395dc87add5b442484abf5ce274921f9ec614f841c8f0630aba61c954afe33a03a06992b876a37381fa03bc3e
languageName: node
linkType: hard

Expand Down Expand Up @@ -6074,7 +6074,7 @@ __metadata:
"@standardnotes/icons": "workspace:*"
"@standardnotes/services": ^1.13.22
"@standardnotes/sncrypto-web": 1.10.1
"@standardnotes/snjs": ^2.118.1
"@standardnotes/snjs": ^2.118.2
"@standardnotes/styles": "workspace:*"
"@standardnotes/toast": "workspace:*"
"@types/jest": ^27.4.1
Expand Down

0 comments on commit d198625

Please sign in to comment.