-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature: Task 6 completed #6
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Привет, посмотрел работу, вот некоторые замечания
Asseccebility:
- Что то пошло не так и все действия с делом не работают(
- nit: Было бы круто переопределить описание click'a у кнопок toolBara, чтобы действие кнопок были понятны для пользователя (например для кнопки меню воспроизводить не коснитесь дважды, чтобы активировать, а коснитесь дважды чтобы перейти в меню приложения; также с глазиком, только там еще стейт меняется).
- nit: считаю что фокус на кнопку добавления нового дела должен попадать раньше, чем на список дел, чтобы пользоваться было удобнее.
- nit: кнопку добавить дело (которая в конце дела) стоит сделать невидимой для talkBack.
- На экране редактирования не очень понятные contentDescription (просто текст) для полей выбора дела, также стоит переопределить описание click'a.
- Свитчер озвучивается неправильно.
- Описание кнопки удалить озвучивается дважды.
Testing:
- UI тесты не запускаются вместе, проходит только addItem. При запуске отдельно - проходят.
- UI тесты есть, но покрытие неполное: не протыкивается выбор дедлайна, приоритета.
Итог:
По доступности замечания есть, но не такие значительные, тяжело учесть все случаи. В тестах большой респект, за большой coverage. В целом работа очень достойная.
@Test | ||
fun addItem() { | ||
CoroutineScope(dispatcher).launch { | ||
delay(1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
задержка избыточна, тк следующей строкой дожидаешься окончания всех корутин. ниже также
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CoroutineExceptionHandler не покрыт тестами
Привет! Спасибо за ревью! По поводу действий с делом: не знаю, почему у тебя не заработало, у меня всё нормально, вот видео: VID_20240728_203114.mp4Напиши пожалуйста, на чём у тебя не заработало, я постараюсь разобраться) |
Запускал на 11 Андроиде, miui 14. |
Шестое задание выполнено.
Сделано по ТЗ
Accessibility
Actions
Actions
на любом элементеАвтотесты