smontlouis / bible-strong

GNU General Public License v3.0
70 stars 7 forks source link

Upgrade React Navigation from v4 to v6 #132

Open smontlouis opened 9 months ago

smontlouis commented 9 months ago

Transitioning our app's navigation system from React Navigation v4 to v6. This upgrade will bring improved performance, latest features, and better compatibility with the latest React Native versions.

Tasks:

  1. Upgrade Dependencies: Update React Navigation and its dependencies to v6.
  2. Refactor Codebase: Adjust navigation code to comply with v6's API changes and best practices.
  3. Test All Navigation Flows: Ensure smooth navigation functionality across the app.
MrScriptX commented 5 months ago

I'm refactoring the code currently, I should be finish soon. I opened the pull request because I might have question on specific files such as createAnimatedBottomTabNavigator.js

About that, is there a specific reason for that file to be a plain JS file. Should I convert it a TSX file ?

smontlouis commented 5 months ago

Hi ! Thank you for your work 👍

The project is 5 or 6 years old, I was not using typescript at the time, so some files are still in js, or using react class components.

I'm not sure, but do you need any .env variables in order to run the project, or are you able to run it without it ?

MrScriptX commented 5 months ago

Hi ! Thank you for your work 👍

You're welcome.

I'm able to run it without .env variables for now but I might need them later on.

MrScriptX commented 5 months ago

I ran into an issue trying to type Link.tsx. While I did succeed in typing in somewhat, the type of params isn't resolving properly from the given route name.

Any idea how to fix this ?

smontlouis commented 5 months ago

Is the problem because we're using a class component? I'll take a look at the code this week-end

MrScriptX commented 5 months ago

I don't think so.

Also, I completed the refactoring (mostly). You can go through testing to check if I didn't miss anything. I'll format everything with the linter also and remove unused code that was commented out.

Type checking is far from perfect but I didn't want to fix it because it would make the pull request way to big. So unless you want to, I'll make the pull request ready for review after you checked everything was working.

smontlouis commented 5 months ago

Size of the PR is not an issue, if you're keen to fix typings I'd be more than happy. Being a little busy at the moment but I'll surely take a look

MrScriptX commented 4 months ago

J'ai trouvé quelques oublies que je suis entrain de corrigé actuellement. Mais je suis tombé sur un problème un peu épineux, peut-être lié à #153 .

Problème 1

Le WebViewQuillEditor remonte des erreurs lors de l'initialisation. De ce que j'ai compris, par moment, l'objet Quill n'est pas initialisé quand le message EDITOR_LOADED est envoyé (et donc quand editorLoaded est appelé). Actuellement, j'ai corrigé cela en appelant editorLoaded via le hook onLoadEnd ce qui règle le problème de chargement des études. Mais, des erreurs sont toujours remontées car dans le WebView, des fonctions de Quill sont appelés alors que ce dernier est nulle.

// WebViewQuillEditor.tsx
 <WebView
      // useWebKit // This doesn't exist anymore
      onLoad={onWebViewLoaded}
      onLoadEnd={() => editorLoaded()} // fix loading issue here
      onMessage={handleMessage}
      originWhitelist={['*']}
      ref={webViewRef}
      source={{ html: studiesHTML }}
      injectedJavaScript={injectFont()}
      domStorageEnabled
      allowUniversalAccessFromFileURLs
      allowFileAccessFromFileURLs
      allowFileAccess
      keyboardDisplayRequiresUserAction={false}
      renderError={renderError}
      onError={onError}
      bounces={false}
      scrollEnabled={false}
      hideKeyboardAccessoryView
      onContentProcessDidTerminate={syntheticEvent => {
          console.warn('Content process terminated, reloading...')
          const { nativeEvent } = syntheticEvent
          webViewRef.current?.reload()
          Sentry.captureException(nativeEvent)
      }}
      onRenderProcessGone={syntheticEvent => {
           webViewRef.current?.reload()
           const { nativeEvent } = syntheticEvent
           Sentry.captureException(nativeEvent)
      }}
/>
// QuillEditor.js
loadEditor = ({ fontFamily, language }) => {
    dispatchConsole(fontFamily)
    document.getElementById('editor').style.fontFamily = fontFamily
    this.quill = new Quill('#editor', {
      theme: 'snow',
      modules: {
        toolbar: BROWSER_TESTING_ENABLED,
        'inline-verse': true,
        'block-verse': true,
        format: true,
      },
      placeholder:
        language === 'fr' ? 'Créer votre étude...' : 'Create your study...',
      readOnly: true,
    })

    dispatchConsole('loading editor')
    this.quill.focus() // this throws error because this.quill doesn't exist yet

    dispatchConsole('editor initialized')
    dispatch('EDITOR_LOADED', {
      type: 'success',
      delta: this.quill.getContents(),
    })
    this.addTextChangeEventToEditor()
  }

Problème 2

J'ai une question qui sera peut-etre un peu plus simple. Actuellement, si on essaie d'ouvrir une étude dans un nouvelle onglet, l'application crash car le studyId est nulle.

// EditStudyScreen.tsx
const studyId = route.params.studyId // doesn't exist
const canEdit = route.params.canEdit
const hasBackButton = route.params.hasBackButton
const openedFromTab = route.params.openedFromTab

Cela s'explique parce qu'on n'utilise pas navigate pour atteindre l'écran et donc, les paramètres n'existent pas. Ce qui est fait actuellement, c'est qu'on utilise dispatchTabs.

// useOpenInNewTab.ts
const openInNewTab = (
    data?: TabItem,
    params: { autoRedirect?: true } = {}
  ) => {
    const newTabId = `new-${Date.now()}`
    dispatchTabs({
      type: 'insert',
      value: {
        id: newTabId,
        title: t('tabs.new'),
        isRemovable: true,
        type: 'new',
        data: {}, // le studyId se trouve ici
        ...data,
      },
    })

    if (!params.autoRedirect) {
      Snackbar.show(t('tabs.created'), 'info', {
        confirmText: t('common.goTo'),
        onConfirm: () => {
          navigation.navigate('AppSwitcher', route.params)
          triggerSlideNewTab(newTabId)
        },
      })
    } else {
      console.log('params', route.params)
      triggerSlideNewTab(newTabId)
      navigation.navigate('AppSwitcher', route.params)
    }
  }

Je ne comprends pas comment je puis récupérer le studyId qui est passé dans le paramètre data du dispatchTabs.

smontlouis commented 4 months ago

Problème 2

Il y a 3 comportements pour le EditStudyScreen

C'est le HOC du EditStudyScreen qui s'occupe de créer le studyId pour l'écran, sois en récupéré donc le param depuis la navigation, depuis les props, ou alors en le créant si c'est une new study

smontlouis commented 4 months ago

Problème 1

Pour l'instant ça ne me parle pas, surtout si les fichiers web du WebViewQuillEditor n'ont pas été touchés, je regarderais

MrScriptX commented 4 months ago

Merci pour le deuxième problème, j'ai pu avancer. En effet, j'avais oublié le HOC et en reportant la logique dans le screen, ça à l'air de fonctionner correctement.