Skip to content

Conversation

@pavelpolischuk
Copy link
Collaborator

No description provided.

@jely-scs
Copy link

Довольно бажно, но если на баги не наступать, то работает)
Баги, из-за которых тяжело:

  1. при переходе на экран с погодой остается клавиатура и от этого разъезжается верстка. Клавиатура при этом скрывается только по back
    На планшете в landscape после скрытия клавиатуры вообще приложение становится неработоспособным - не работает ни поиск, ни экран не разворачивается.
  2. на планшете не работает кнопка настроек (чтобы перейти туда нужно повернуть планшет)
  3. Можно попасть в состояние с неработоспособным поиском. Шаги:
  • с открытой клавиатурой нажать на настройки (в момент набора города вдруг решил поменяь цельсий на фаренгейт)
  • (захотел вернуться по системному back-у) нажать back 2 раза (скрыть клавиатуру, вернуться на предыдущий экран)
  • попадаешь в состояние, что не работает поиск
    Менее заметно:
  1. на странице с погодой плохо скроллится recycle view из-за конфликта с pull-to-refresh
  2. в лендскейпе на странице погоды разъезжается верстка и не скроллится
  3. время обновления пишется из будущего
    Ну и полностью проигнорированы задания про дизайн.

@jely-scs
Copy link

jely-scs commented Aug 17, 2017

В целом, радует, что есть осмысленное разделение по пакетам (хотя лучше разделение по фичам-экранам, чем просто так), зависимости через даггер. Хорошо бы подпричесать код, убрать то, что сейчас уже не используется.
А еще, на дизайн приложения лучше совсем не забивать, поэтому что первое впечатление все равно идет от него.
Круто, когда тесты не ради покрытия, а осмысленны (но стоит отдельно обращать внимание, чтобы они не были просто копией кода с verify-ми)

@jely-scs
Copy link

Еще можно добавить аннотаций Nullability, облегчит жизнь

@pavelpolischuk
Copy link
Collaborator Author

Согласен со всем, баги и дизайн конечно хотелось поправить, так много проблем потому что логика ui делалась последние 1,5 дня. Проверялось на скорую руку вечером в воскресенье. Многие баги я сам обнаружил, но есть дедлайн(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants