artsiom-voitas/github-dashboard

GitHub Dashboard

Search for repositories on GitHub
View Demo

Report Bug · Request Feature

About The Project

       

This application is built on the GitHub API. With it, you can:

• get a list of the ten most popular repositories on GitHub
• perform a repository search via a query
• view the repository card (owner, owner's photo, date of last commit, description, most popular contributors)

Built With

• 
• 

Contact

Artsiom Voitas - @artsiom_voitas - [email protected]

Project Link: https://github.com/artsiom-voitas/github-dashboard

Acknowledgments

• Artsiom Dadychanka - teacher, mentor

github-dashboard's Issues

Review

https://github.com/artyom-voitas/github-dashboard/blob/master/README.md

Readme файл должен содержать полную информацию о проекте. Это важно, в случае если перед прохождением собеседования попросят показать Github

https://www.mygreatlearning.com/blog/readme-file/#:~:text=readme%20file%20is.-,What%20is%20a%20README%20File%3F,about%20the%20patches%20or%20updates.

---

Некоторые зависимости должны быть перенесены в devDependencies секцию

https://github.com/artyom-voitas/github-dashboard/blob/eb2133d7c0655073db9c9b7f5c7be7cdc05da580/package.json

 "@testing-library/jest-dom": "^5.16.5",
    "@testing-library/react": "^13.3.0",
    "@testing-library/user-event": "^13.5.0",
    "@types/react": "^18.0.17",
    "gh-pages": "^4.0.0",
        "cross-env": "^7.0.3",

---

https://github.com/artyom-voitas/github-dashboard/blob/eb2133d7c0655073db9c9b7f5c7be7cdc05da580/src/App.js

Не хватает React.memo для App компонента

Если используются index.js файлы для папок, то импорты можно сократить

import SearchQuery from "./components/SearchQuery/index"; => import SearchQuery from "./components/SearchQuery";

---

https://github.com/artyom-voitas/github-dashboard/blob/eb2133d7c0655073db9c9b7f5c7be7cdc05da580/src/components/Main/Main.jsx

Нет никакой проблемы в том, чтобы включить dispatch в список зависимостей

useEffect(() => {
    dispatch(getMostPopularRepositories());
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

https://stackoverflow.com/questions/56795607/what-are-the-cases-where-redux-dispatch-could-change

---

Слова reducer не должно быть в стейте

(state) => state.mostPopularRepositoriesReducer.items

---

Если аргумент не используется, его можно удалить.

function Main(_props) {

---

Можно сократить импорт, в папке RepositoriesList есть index.js

import RepositoriesList from "../RepositoriesList/RepositoriesList"; => import RepositoriesList from "../RepositoriesList";

---

Эта ветка условия никогда не отработает, нету ситуации где repos будет null или undefined

{!repos ? (
        <ErrorMessage />
      ) : (

---

Выглядит как div ради div'a, можно заменить на Fragment

  <div>
          <Header />

---

https://github.com/artyom-voitas/github-dashboard/blob/eb2133d7c0655073db9c9b7f5c7be7cdc05da580/src/components/ErrorMessage/ErrorMessage.jsx

Никаких инлайн стилей. Есть бутстрап классы для этого https://getbootstrap.com/docs/5.2/utilities/position/
Если нету, то нужно создать свой класс

<div
        style={{
          position: "absolute",
          left: "50%",
          top: "50%",
          transform: "translate(-50%, -50%)",
        }}
      >

style={{
            minWidth: 320,
            textAlign: "center",
          }}

---

В alt аттрибуте пишется текстовое описание объясняющее изобращение, которое отображается

alt="ErrorMessage"

---

Нету React.memo

---

https://github.com/artyom-voitas/github-dashboard/blob/eb2133d7c0655073db9c9b7f5c7be7cdc05da580/src/components/Header/Header.jsx

Если не нужны пропсы, нужно их убрать

function Header(_props)

---

Очень странное форматирование в этом файле, отличное от других, нужно привести всё к одному

---

1. Функция будет пересоздаваться при каждом ререндере компонента, нужно обренуть в useCallback
2. Обработчики событий обычно называются начиная со слова handle onChange={handleChange}

function onInputChange(event)
  {
    setSearchValue(event.target.value)
  }

function searchHandler() {
    dispatch(getRepositoriesBySearchQuery(searchValue, currentPage))
  }

---

Строку можно сразу на placeholder инпута повесить

const searchPlaceholder = "Search or jump to...";

---

Нету React.memo

---

Если пишешь aria-label то пиши их правильно

 <Form.Control
          aria-label="Text input with dropdown button"

https://www.aditus.io/aria/aria-label/#:~:text=Over%2Dusing%20it-,Definition,content%20as%20the%20accessibility%20label.

---

Review #2

https://github.com/artyom-voitas/github-dashboard/blob/c8b61232ae2436f4c797148c0822df6d7d671d91/package.json

Это команды, которые будут запускаться из терминала. Их нужно поместить в секцию scripts

   "lint": "eslint .",
    "lint:fix": "eslint --fix",
    "format": "prettier --write './**/*.{js,jsx,ts,tsx,css,md,json}' --config ./.prettierrc",

---

На windows не отрабатывает команда "format": "prettier --write './**/*.{js,jsx,ts,tsx,css,md,json}' --config ./.prettierrc"

[error] Invalid configuration file .eslintrc.js: ENOENT: no such file or directory, open 'D:\Other\G2\React final projects\github-dashboard.prettierrc'

---

https://github.com/artyom-voitas/github-dashboard/blob/c8b61232ae2436f4c797148c0822df6d7d671d91/src/components/Main/Main.jsx

Для фрагментов есть более удобный синтаксис, который не требует импорта из библиотеки React

<Fragment> ... </Fragment => <>...</>

---

https://github.com/artyom-voitas/github-dashboard/blob/c8b61232ae2436f4c797148c0822df6d7d671d91/src/components/RepositoriesList/RepositoriesList.jsx

Можно тип репозитория расписать более подробно (только то, что используешь в коде) через arrayOf.

repositories: PropTypes.array.isRequired,

---

Аргумент в коллбеке называется repos, хотя в нём лежит один репозиторий, логичнее назвать repo

repositories.map((repos) => {

---

https://github.com/artyom-voitas/github-dashboard/blob/2281540c23bc8149617ac943cdf75f5a6a38c6af/src/components/SearchItemCard/SearchItemCard.jsx

Обработчики нужно оборачивать в useCallback, чтобы функции не создавались заново каждый ререндер

function handleClick() {
    dispatch(getRepository(owner, title));
    dispatch(getRepositoryStargazers(owner, title));
  }

Лучше обернуть в useMemo, чтобы избежать лишних ненужных вызовов функции formatLastCommitDate при ререндере.
Если бы это было обычное примитивное значение, то можно не оборачивать.

const formattedLastCommitDate = formatLastCommitDate(lastCommitDate);

---

https://github.com/artyom-voitas/github-dashboard/blob/2281540c23bc8149617ac943cdf75f5a6a38c6af/src/components/SearchQuery/SearchQuery.jsx

Это тоже лучше обернуть в useMemo

const { query, page } = queryString.parse(search);

---

{/* eslint-disable-next-line react/no-unescaped-entities */}

Так не пойдет. Почитай про само правило https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/no-unescaped-entities.md

Можно заменить

We haven't found anything on your request: => {"We haven't found anything on your request:"}

---

Можно упростить

{!isLoading ? <SearchPagination query={query} currentPage={pageNumber} /> : null} => {!isLoading && <SearchPagination query={query} currentPage={pageNumber} />}

---

https://github.com/artyom-voitas/github-dashboard/blob/2281540c23bc8149617ac943cdf75f5a6a38c6af/src/components/SearchPagination/SearchPagination.jsx

Можно вынести из компонента, функция использует только аргументы. Лучше назвать getTotalPagesAmount. 100 - magic number, можно вынести в константу и дать название.

const countTotalPages = function (totalCount, reposPerPage) {
    const pagesAmount = Math.ceil(totalCount / reposPerPage);
    if (pagesAmount > 100) {
      return 100;
    } else {
      return pagesAmount;
    }
  };

---

Можно добавить параметр query и вынести в функцию вне компонента. Плохое название, потому что точно твоя функция ничего не устанавливает (set), лучше getLinkToPage

function setUrl(page) {
    return `/github-dashboard/#/search?query=${query}&page=${page}`;
  }

---

Дальше в разметке много magic numbers, нужно вынести в константы и дать осмысленные названия.

---

Можно сделать условным не разметку а проп disabled

{currentPage === pagesAmount ? (
        <Pagination.Next disabled />
      ) : (
        <Pagination.Next href={setUrl(currentPage + 1)} />
      )}

=>

<Pagination.Next disabled={currentPage === pagesAmount} href={setUrl(currentPage + 1)} />

---

Много похожих кусков можно записать без тернарного оператора

{currentPage === 1 && pagesAmount >= 5 ? (
        <Pagination.Item href={setUrl(currentPage + 4)}>{currentPage + 4}</Pagination.Item>
      ) : null}

=>

{currentPage === 1 && pagesAmount >= 5 && (
        <Pagination.Item href={setUrl(currentPage + 4)}>{currentPage + 4}</Pagination.Item>
      )}

---

Review #3

https://github.com/artyom-voitas/github-dashboard/blob/2281540c23bc8149617ac943cdf75f5a6a38c6af/src/components/RepositoryCard/RepositoryCard.jsx

useMemo

const tenTopStargazers = stargazers.slice(0, 10);

---

if проверку можно вынести в функцию formatLastCommitDate

let formattedLastCommitDate;
  if (commitDate) {
    formattedLastCommitDate = formatLastCommitDate(commitDate);
  }

---

Можно использовать деструктуризацию, чтобы не дублировать название.

const description = repository.description; => const { description } = repository;

---

Если перейти на страницу несуществующего репозитория, приложение крашится. Ищи ошибку в repositoryStargazersReducer

/github-dashboard#/repository/inamiy/SwiftRewriter22

Uncaught TypeError: stargazers.slice is not a function

---

Нужно выстроить код таким образом, чтобы уйти от деталей реализации/ответа от гитхаб сервера в коде компонента.

if (serverRespond === 'Not Found') {

https://github.com/artyom-voitas/github-dashboard/blob/2281540c23bc8149617ac943cdf75f5a6a38c6af/src/redux/repositoryCardReducer/thunk.js

.then((result) => {
        dispatch(getRepositorySuccess(result));
      })

Тут можно реализовать дополнительную проверку на message поле и диспатчить getRepositoryError, вместо getRepositorySuccess с полем message

---

Слишком много условий в коде компонента, можно упростить их и сделать только для текста или классов.

Условия можно упростив убрав тернатный оператор и добавив логические операторы.

{description && <div>Some content</div}
{!description && <div>Placeholder</div>}

Такой вариант более читабельный и понятный

---

Кусок стора называется items, в нём лежит объект с один репозиторием, нужно переделать.

const repository = useSelector((state) => state.repositoryCard.items);

---

При вводе поискового запроса на страницу карточки 1 репозитория поиск происходит по значение поискового запроса в инпуте поиска не сохраняется