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

Internal-270 Feature base classes and view state #249

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Koshake
Copy link

@Koshake Koshake commented Mar 23, 2022

https://jira.touchin.ru/browse/INTERNAL-270

Добавлен базовый функционал для дальнейшей работы с отображение PDF файлов
Добавлен функционал для конвертации файла в Bitmap для дальнейшего использования в ImageView

Необходимо реализовать базовый функционал для отображения PDF-файлов:

В итоговой реализации должен быть RecyclerView, а внутри него ImageView. Суть в том, что PDF файл надо сконвертировать в Bitmap и положить в ImageView.

На данный момент реализована базовая ViewModel для работы с состояниями: успешная успешная отрисовка, ошибка, загрузка. В BaseViewModel также добавлен функционал для работы с корутинами (скоуп и билдер)
В PdfViewModel содержатся методы:

  1. для асинхронной загрузки PDF-файла из сети
  2. для конвертации в Bitmap

Для конвертации в Bitmap используются методы класса PdfRenerer из android.graphics.pdf

Comment on lines +1 to +10
plugins {
id 'com.android.library'
id 'org.jetbrains.kotlin.android'
}

android {
compileSdk 31

defaultConfig {
minSdk 23
Copy link
Contributor

Choose a reason for hiding this comment

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

Скрипты уже есть в android-configs/lib_config.gradle
Можешь просто прописать apply from: "../android-configs/lib-config.gradle"

Comment on lines +3 to +10
import android.graphics.Bitmap
import java.io.File

interface PdfViewRepository {

suspend fun renderSinglePage(filePath: String, width: Int) : Bitmap

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Не совсем понятен функционал этого Repository. Не стоит вытаскивать логику рендеринга и взаимодействия с ресурсами андроида на Model-уровень

Comment on lines +11 to +15

abstract class BaseFragment<VM : BaseViewModel> : Fragment() {

abstract val viewModel: VM

Copy link
Contributor

Choose a reason for hiding this comment

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

Лучше использовать уже готовую реализацию фрагментов и ViewModel. Я бы посоветовал в этот модуль добавить зависимость от mvi_arch и реализовать Fragment и ViewModel, как наследников MviFragment и MviViewModel соответственно.

Можешь почитать про архитектуру вот тут - https://github.com/TouchInstinct/Styleguide/blob/master/Android/Arch3.md

).show()
}

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Не стоит забывать прогонять линтеры. Тут например Detekt упадет, т.к. нет пустой строчки в конце

Choose a reason for hiding this comment

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

а можно сделать так, чтобы они сами автоматически запускались?

Copy link
Contributor

Choose a reason for hiding this comment

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

Пока не вижу в этом очевидных проблем. Теоретически можно плагин натравить на каждый из модулей в робосваге. Надо немного потрогать настройки StaticAnalysisAndroidPlugin

protected val mStateLiveData = MutableLiveData<PdfReaderViewState>()
val stateLiveData get() = mStateLiveData as LiveData<PdfReaderViewState>

private val viewModelCoroutineScope = CoroutineScope(
Copy link
Contributor

Choose a reason for hiding this comment

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

Так ведь есть уже viewModelScope в реализации ViewModel

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.

3 participants