14 Matching Annotations
  1. Nov 2018
    1. classes: PropTypes.shape().isRequired,
      PropTypes.object.isRequired
      
    2. <CardActionArea href={`/${username}/${repo}/${number}`} target="_blank">

      Il y a plusieurs problème ici:

      1. C'est un presentational component. Ce n'est pas sa tâche de choisir l'url. Du coup il n'est pas réutilisable. Tu devrait soit lui passer l'url par props ou laisser le composant parent lui gérer cela.

      2. Tu utilise react-router dans ton app. Tu devrais utiliser le compose Link de react-router.

      import { Link } from 'react-router-dom'
      
      <Link to={`/${username}/${repo}/${number}`}>
      
      1. target="_blank" force l'ouverture de l'URL dans une nouvelle tab. C'est à l'utilisateur de choisir ce qu'il veut faire. Surtout que c'est URL interne à ton app.
    3. <TableRow key={row.cursor} style={{ padding: 0 }}> <TableCell style={{ padding: 0 }}> <TableIssueItem title={row.node.title} number={row.node.number} createdAt={row.node.createdAt} state={state} login={row.node.author.login} username={username} repo={repo} /> </TableCell> </TableRow>

      Pourquoi utiliser une Table si c'est juste pour utiliser une seul colonne ?

    4. const { edges } = data.repository.issues; return ( <TableIssues datas={edges} username={username} repo={repo} state={state} /> );

      Mieux vaut passer une liste d'issues. C'est beaucoup plus clair pour le composant TableIssues.

      const issues = edges.map(edge => edge.node)
      return <TableIssues data={issues}>
      
    5. classes: PropTypes.shape().isRequired,
      PropTypes.object
      

      ou

      PropTypes.objectOf(PropTypes.string),
      

      J'ai créé un raccourci pour ça dans mes projets.

    6. datas: PropTypes.instanceOf(Array).isRequired,

      Alternatives:

      PropTypes.array
      
      PropTypes.arrayOf(PropTypes.object)
      

      De préférence celle-ci. Car ta structure de donnée est compliquée.

      PropTypes.arrayOf(PropTypes.shape({
        cursor: PropTypes.string,
        node: PropTypes.shape({
          title: PropTypes.string,
          number: PropTypes.number,
          // etc..
        }),
      })
      
    7. search

      search() -> handleSearch()

      Avec React, c'est une convention d'appeler ce genre de méthode: handleX lorsqu'on réagit à un event par exemple.

    8. import MainPage from './Components/MainPage'; import './App.css'; import IssueDetailsContainer from './Components/IssueDetails/IssueDetailsContainer';

      MainPage et IssueDetailsContainer sont des pages. Le fait que ces composants soient dans des dossiers complètement différents et portent des nom différents porte un peu la confusion.

      Si je dois rajouter une nouvelle page dans ton app. Je nomme comment le composant ? Je le met où ?

    9. "eslint": "^5.6.0", "eslint-config-airbnb": "^17.1.0", "eslint-plugin-import": "^2.14.0", "eslint-plugin-jsx-a11y": "^6.1.2", "eslint-plugin-react": "^7.11.1"

      create-react-app inclus déjà eslint. En l'installant manuellement comme tu l'a fait ça pose des problèmes de version.

      Une meilleure solution pour pouvoir ajouter tes propres rules eslint: Ajouter uniquement le fichier .eslintrc. Read more

    1. new User

      Ce n'est pas une bonne idée d'utiliser des classes avec React. Il faut utiliser soit des objets ou des classes immutable (https://facebook.github.io/immutable-js/)

    2. ReactDOM.render( <AppIssue repo={repo} currentUser={amadeous} />, document.getElementById('App') ); ReactDOM.render( <GithubHeader user={amadeous} />, document.getElementById('Header') );

      Éviter d'utiliser plusieurs render à la fois car il ne sera difficilement possible de partager des information entre les différents composants