Насколько решение соответствует ТЗ(React)? Есть ли грубые ошибки?

Question

[Александр Буки]

Уважаемые специалисты по React, буду очень признателен, если кто-то посмотрит как было выполнено тестовое задание. Сроки довольно сжатые и я все время работал на vue js + vuex.
Как показала практика он сильно отличается от react + redux.
Понимаю что дорабатывать там много всего нужно, важно указать на грубые ошибки.

Текст задания:
Реализовать на реакте интерфейс быстрого поиска. Состоит из двух частей.
1. Верхняя панель: поисковая строка с инпутом. В процессе ввода букв запроса, появляются и конкретизируются саджесты под инпутом и результаты.
2. Результаты: список из 10 результатов + пагинация. Параметры в урл добавлять не требуется.
(я понял, что пагинация на клиенте)
Быстрый поиск означает, что результаты обновляются в процессе ввода запроса в поисковую строку. Для хранения минимального состояния приложения (текущая страница пагинации, текущий запрос) нужно использовать redux. Бэкенд не нужен (все моки данных можно хранить прямо в json-файле, читать прямо оттуда), дизайн не требуется.

Важно продемонстрировать умение декомпозировать интерфейс на компоненты, давать им понятные имена.

У меня получился следующий код:
https://codesandbox.io/s/github/vito2005/mrtest

https://ctm1w.csb.app/

- данные забираю из json через промисы и сет таймаут
- json - массив городов с адресами и координатами
- делаю поиск в json по началу названия города
- компоненты самые простые: 1. компонент автокомплит для suggestions; 2. компонент контент для результатов; 3. пагинация внутри контента
- два асинхронных экшн в редакс для запроса suggestions и результатов

Comments

[Павел Антонов]

В Content.js
Я бы конвертировал класс в функцию, т.к не используется state и методы жизненного цикла.

Здесь this.props.loading можно убрать в деструктуризацию, раз уж используете.

render() {
    const {
      results, currentPage, resultsPerPage, setCurrentPage
    } = this.props;
    ....
    ....
    if (this.props.loading) {
      return (<div className="loading-spinner">...loading</div>);
    }
  }

[davidnum95]

бинд в конструкторе - моветон, используйте стрелочные функции

Answer 1

[Афанасий Захаров]

По коду кажется все ОК (никаких сильных претензий не возникает сходу), только по-моему пагинацию прикрутить вы забыли (она нигде не вызывается).

UPD: претензии небольшие все таки есть, class Results:

renderNoResultsMessage() {
    const { results, noResultsMessage } = this.props;
    if (results && !results.length && noResultsMessage) {
      return (<div className="no-result">
        <h3>
          {noResultsMessage}
        </h3>
      </div>);
    }
  }

Кажется эта функция не очень то renderNoResults, она еще checkForResults, а только потом render - не надо так делать, по коду непонятно. функция, которая renderSMTH -должна только рендерить, без лишних проверок

UPD 2 пагинацию увидел, с ней все ок.

Comments

[Александр Буки]

Спасибо за ответ. Судя по заданию (Пункт 2: ...Параметры в урл добавлять не требуется) пагинацию я сделал на клиенте, но этот момент уточняю. Просто не понимаю как получать данные постранично, если их в запросе не указывать

[Александр Буки]

по render no results +++

[Афанасий Захаров]

Александр Буки еще один момент, меня он удивил, если это тестовое - так же может вызвать вопрос -

const cities = await loadSuggestions(value);
    const isDataEmpty = cities.length === 0;
    if (cities.length > 9) cities.splice(10, cities.length); // check length of result
    if (isDataEmpty) dispatch({ type: HIDE_SUGGESTIONS });

Зачем вы тут отрезаете остаток, если больше 10 саджестов? это как то указано в ТЗ?
Если нет, проще оставлять все, ИМХО.

[Александр Буки]

Афанасий Захаров, не указано, чисто визуально подумал будет лучше. Думаю это не критичный момент

[Афанасий Захаров]

и еще момент, связанный с построением папок - внутри store у вас одна пара actions/reducer лежит на верхнем уровне, одна - на уровень пониже, в папке - почему вы так решили?
ИМХО, стоит обе пары положить на один уровень (каждую в свою папку), а combineReducers сделать в файле index.js внутри папки store (больше index.js ничего делать не будет).

[Александр Буки]

Афанасий Захаров, по сути да, я так и хотел, но не понял куда девать экшн, который забирает результаты, поэтому оставил пока так

[Александр Буки]

Афанасий Захаров, честно говоря я с redux помучился, vuex довольно сильно отличается по построению взаимосвязей

[Афанасий Захаров]

Александр Буки, все, я вас понял.
Есть у вас тут критическая ошибка.
Отсутствие инкапсуляции - у вас компонент Autocomplete вообще ничего не должен знать про getDataInjector.
он должен наверх, в App пробрасывать результат выбора пользователя, и уже там, в компоненте App эта штука должна обрабатываться и выводиться данные.

[Александр Буки]

Афанасий Захаров, согласен, единственное что сейчас спасает - это делается через redux.
Работу уже сдал, может быть опустят на это глаза

[Александр Буки]

Афанасий Захаров, заметил недостаток:
когда появляются suggestions нельзя по ним бегать стрелочками с клавиатуры.
Подскажете как это делается в общих словах?

[Афанасий Захаров]

Александр Буки, надо ловить событие onkeypress, вычислять нужную клавишу (вверх-вниз), саджесты рендерить в отдельном компоненте, и передавать управление в него, где выделенный саджест подсвечивается например синим.

[Александр Буки]

Афанасий Захаров, спасибо!

[Александр Буки]

Афанасий Захаров, пытаюсь доработать задание.
Я вас понял про инкапсуляцию автокомплита, только я не знаю как лучше наверх пробросить изменение состояния инпута(пользователь нажал поиск)?
Во вью есть event emitter
А как в реакте?
Или как-то через redux это все делать?

[Афанасий Захаров]

Александр Буки, внутрь автокомплита в качестве пропса пробрасываете функцию onSelected (ну, называть можете как хотите), которую описываете в основном компоненте. Ее вызываете из автокомплита, передавая выделенное пользователем значение.