Closed CloudLevi closed 3 years ago
Additionally, fixed ARK-Builders/ARK-Navigator#93. It was a flaw that happened during refractoring to ViewBinding.
Maybe we should create a
PreviewRepo
that can be delivered via DI, can generate previews and provide them?
To me, seems like overkill for this kinda task. Are there many more places where we'll implement previews?
Maybe we should create a
PreviewRepo
that can be delivered via DI, can generate previews and provide them?To me, seems like overkill for this kinda task. Are there many more places where we'll implement previews?
If we are implementing "Link to Web" resource type, then I think we will need to show a preview before adding a resource on another screen.
We can also keep a cache for the preview paths in the application lifecycle instead of looking for previews in the file system every time.
Made PreviewRepo. Not sure how much of the logic is intented to be moved to the PreviewRepo. What are your thoughts? For example, function createPdfPreview - seems like it still should stay in FileUtils.
@mdrlzy
If we are implementing "Link to Web" resource type, then I think we will need to show a preview before adding a resource on another screen.
Could you elaborate this?
I've added getMeta()
method into ResourcesIndex
and also modified ResourceMeta
to contain type-specific data. Sorry, I had no time to implement type-specific data itself, so I put stubs (see //TODO: PR ARK-Builders/ARK-Navigator#33
in the code). The idea is that we have to generate metadata like video duration/resolution during indexing and save it into extra
field.
@CloudLevi can we re-design previews the way we would not need to store FileType
in Preview
?
I think something is wrong here, because we dispatch on the type in classes like FileItemViewHolder
and PreviewsList
but they should be simple and operate with primitive images already without knowledge about resources at all. Preview should be created basing on resource type, it should not carry it further.
Work moved to https://github.com/ARK-Builders/ARK-Navigator/pull/122
Also, this PR should solve the issue ARK-Builders/ARK-Navigator#81. The issue was quite inconsistent when I tested it, but it seems like on this branch it doesn't appear.