Skip to content
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

Iteration 1 review #31

Merged
merged 64 commits into from
Aug 5, 2024
Merged

Iteration 1 review #31

merged 64 commits into from
Aug 5, 2024

Conversation

G-dev-ui
Copy link
Collaborator

@G-dev-ui G-dev-ui commented Aug 5, 2024

https://github.com/users/ElchinGasymov/projects/1/views/1
Ссылка на доску
Все поставленные задачи Epic0 выполнены
Количество людей в команде: 5
Первоначальная оценка времени 5 дней, затраченной время 5 дней

ElchinGasymov and others added 30 commits August 2, 2024 10:31
Добавлен фрагмент "Вакансия" экрана описания вакансии
Create develop.properties
Исправлена ошибка в layout vacancy_fragment
Изменен родительский стиль темы приложения с Theme.Material3.DayNight.NoActionBar на Theme.MaterialComponents.DayNight.NoActionBar
Добавлен строковый ресурс для отображения количества найденных вакансий.
Изменен родительский стиль темы приложения с Theme.Material3.DayNight.NoActionBar на Theme.MaterialComponents.DayNight.NoActionBar
Добавлен строковый ресурс для отображения количества найденных вакансий.
Изменен манифест и градл
Добавил круглую и квадратную легаси иконки
Добавлены цвета, темы, строковые ресурсы
…_fonts

Добавлены изображения, шрифты,  размеры
Logomann and others added 26 commits August 3, 2024 14:48
…and_checking-Internet

Добавил утилиты для дебаунса и проверки интернета
…_and_screen_caps

Граф нафигации и основные экраны
…ation_view

Добавил панель навигации, выпилил ненужный второй синий цвет
# Conflicts:
#	app/src/main/java/ru/practicum/android/diploma/di/DataModule.kt
Выполнена верстка экрана Команды.
Добавил все необходимое и немного подправил градл для анатаций
…n_fix

Починил bottomNavigationView
Comment on lines +37 to +38
retrofit = { module = "com.squareup.retrofit2:retrofit", version.ref = "retrofit" }
retrofit-v290 = { module = "com.squareup.retrofit2:retrofit", version.ref = "retrofitVersion" }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Зачем вам две версии одной и той же библиотеки? Лучше использовать только 1, градл так или иначе зарезолвит вам конфликт версий, но тем не менее вручную не стоит добавлять две версии одной зависимости

import ru.practicum.android.diploma.data.dto.Responce

interface NetworkClient {
suspend fun doRequest(dto: Any): Responce

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
suspend fun doRequest(dto: Any): Responce
suspend fun doRequest(dto: Any): Response

Опечатка


class FavouritesFragment : Fragment() {

private val binding: FragmentFavouritesBinding by viewBinding(CreateMethod.INFLATE)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

При использовании viewBinding во фрагментах обязательно нужно обнулять _binding в onDestroyView, иначе будет утечка памяти. Почитать о том, как использовать эту фичу во фрагментах можно тут - https://developer.android.com/topic/libraries/view-binding#fragments. Почитать подробнее про утечку можно здесь - https://stackoverflow.com/questions/65295104/android-view-binding-clear-binding-in-fragment-lifecycle

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ко всем фрагментам это относится

Comment on lines +30 to +44
R.id.mainFragment -> {
binding.bottomNavigationView.isVisible = true
}

R.id.favoriteFragment -> {
binding.bottomNavigationView.isVisible = true
}

R.id.teamFragment -> {
binding.bottomNavigationView.isVisible = true
}

else -> {
binding.bottomNavigationView.isVisible = false
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Объедините здесь лучше в две строчки, типа такого:

R.id.mainFragment, R.id.favoriteFragment, R.id.teamFragment -> binding.bottomNavigationView.isVisible = true
else -> binding.bottomNavigationView.isVisible = false

var debounceJob: Job? = null
setOnClickListener {
debounceJob?.cancel()
debounceJob = CoroutineScope(Dispatchers.Main).launch {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Передайте скоуп в параметрах функции, не стоит на каждую корутину новый скоуп создавать

android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/dimen_8dp"
android:text="Перейти к фильтрам"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Строчки все необходимо в ресурсы вынести

super.onViewCreated(view, savedInstanceState)

binding.buttonBackToSearchFromFilter.setOnClickListener {
findNavController().navigate(R.id.mainFragment)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут либо popBackStack() должен быть, либо navigateUp(), т.к. тут нужно на предыдущий фрагмент вернуться, а не на нвоый экран переходить

@G-dev-ui G-dev-ui merged commit 3d6834d into main Aug 5, 2024
3 checks passed
@ElchinGasymov ElchinGasymov deleted the iteration_1_review branch August 16, 2024 07:43
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.

6 participants