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

Question

Александр Буки @alexbuki

программист js

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

Уважаемые специалисты по 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 и результатов

Вопрос задан более трёх лет назад
794 просмотра

2 комментария

Подписаться 8 Средний 2 комментария

Павел Антонов @freislot
В 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 @davidnum95

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

Написано более трёх лет назад

Решения вопроса 1

14 комментариев

Александр Буки @alexbuki Автор вопроса

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

Написано более трёх лет назад
Александр Буки @alexbuki Автор вопроса

по render no results +++

Написано более трёх лет назад
Афанасий Захаров @afanasiyz
Александр Буки еще один момент, меня он удивил, если это тестовое - так же может вызвать вопрос -
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 саджестов? это как то указано в ТЗ?
Если нет, проще оставлять все, ИМХО.
Написано более трёх лет назад
Александр Буки @alexbuki Автор вопроса

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

Написано более трёх лет назад
Афанасий Захаров @afanasiyz

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

Написано более трёх лет назад
Александр Буки @alexbuki Автор вопроса

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

Написано более трёх лет назад
Александр Буки @alexbuki Автор вопроса

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

Написано более трёх лет назад
Афанасий Захаров @afanasiyz

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

Написано более трёх лет назад
Александр Буки @alexbuki Автор вопроса

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

Написано более трёх лет назад
Александр Буки @alexbuki Автор вопроса

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

Написано более трёх лет назад
Афанасий Захаров @afanasiyz

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

Написано более трёх лет назад
Александр Буки @alexbuki Автор вопроса

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

Написано более трёх лет назад
Александр Буки @alexbuki Автор вопроса

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

Написано более трёх лет назад
Афанасий Захаров @afanasiyz

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

Написано более трёх лет назад

Пригласить эксперта

Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации

Похожие вопросы

HTML

+1 ещё

Простой
Почему не работает pattern на input?
- 1 подписчик
- 14 часов назад
- 58 просмотров
2

ответа
React

Простой
Почему не рендерится React-компонент в методе render( )?
- 1 подписчик
- 15 часов назад
- 69 просмотров
1

ответ
React

Простой
У меня react up и я беру из .env пароль а мне бьет ошибку, как быть?
- 1 подписчик
- вчера
- 56 просмотров
0

ответов
JavaScript

+1 ещё

Средний
Сдвигается нижняя граница во время поиска Как исправить?
- 1 подписчик
- 16 апр.
- 46 просмотров
0

ответов
JavaScript

+4 ещё

Простой
Что делать, если после залива приложения на VPS страница остается недоступной?
- 1 подписчик
- 15 апр.
- 84 просмотра
2

ответа
React

Простой
В какую папку React компилирует все файлы?
- 1 подписчик
- 13 апр.
- 82 просмотра
2

ответа
React

+1 ещё

Средний
При стилизации компонента React не видны scss переменные подключенные модулем к index.js, в чем может быть проблема?
- 2 подписчика
- 13 апр.
- 316 просмотров
1

ответ
React

+1 ещё

Простой
Ошибка Cannot read properties of null (reading 'useContext')?
- 1 подписчик
- 13 апр.
- 40 просмотров
0

ответов
React

Простой
Почему при клике на навигацию не меняется id?
- 1 подписчик
- 13 апр.
- 41 просмотр
1

ответ
React

+1 ещё

Средний
Как сделать асинхоронным dispatch?
- 1 подписчик
- 13 апр.
- 28 просмотров
0

ответов
Показать ещё Загружается…

Frontend React

Glob-com

от 100 000 ₽

JS|React Native разработчик

Венста • Киров

от 60 000 до 120 000 ₽

Frontend-разработчик (React)

ДАЛЕЕ

от 130 000 ₽

Обработать массив фотографий

19 апр. 2024, в 08:46

5000 руб./за проект

Скрыть / убрать лишние поля в форме заказа

19 апр. 2024, в 07:30

1500 руб./в час

Взлом автомобильной программы

19 апр. 2024, в 05:01

999999 руб./за проект

В 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>); } }
бинд в конструкторе - моветон, используйте стрелочные функции

Answer 1 · 2019-11-11 11:08:43

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

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 пагинацию увидел, с ней все ок.

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

Войдите, чтобы написать ответ

Минуточку внимания

Войдите на сайт