Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade React Navigation from v4 to v6 #132

Closed
smontlouis opened this issue Feb 3, 2024 · 11 comments · Fixed by #161
Closed

Upgrade React Navigation from v4 to v6 #132

smontlouis opened this issue Feb 3, 2024 · 11 comments · Fixed by #161

Comments

@smontlouis
Copy link
Owner

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
Copy link
Collaborator

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
Copy link
Owner Author

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Owner Author

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

@MrScriptX
Copy link
Collaborator

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
Copy link
Owner Author

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
Copy link
Collaborator

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
Copy link
Owner Author

Problème 2

Il y a 3 comportements pour le EditStudyScreen

  • On peut ouvrir le screen dans une stack classique depuis le StudiesScreen qui va donner un studyId dans le param à travers le lien de navigation

  • On peut ouvrir le screen dans une stack classique en appuyant sur le "nouvelle étude" depuis le StudiesScreen sans passer de studyId, mais en fournir un à travers le HOC en ligne 47.

  • On peut ouvrir le screen dans une tab à travers la méthode dispatchTabs depuis le hook useOpenInNewTab, utilisé par src/features/studies/EditStudyHeader.tsx et qui va donner studyId dans les data de la tab. Ce studyId param est reçu en props par le EditStudyScreen, car il a été passé depuis le StudiesTabScreen. En gros le StudiesTabScreen affiche la liste des études si pas de studyId dans son data, et affiche une étude quand il a un studyId

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
Copy link
Owner Author

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
2 participants