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

Update android side #68

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

Conversation

alibek228k
Copy link

No description provided.

@sonarcloud
Copy link

sonarcloud bot commented Apr 20, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@Alex009 Alex009 changed the base branch from master to develop April 20, 2023 07:23
Copy link
Member

@Alex009 Alex009 left a comment

Choose a reason for hiding this comment

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

also CI should be fixed.
use ./gradlew build to test


package dev.icerock.moko.media

expect fun FileMedia.toByteArray(): ByteArray
Copy link
Member

Choose a reason for hiding this comment

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

revert please. this change of public api

@@ -6,5 +6,6 @@ package dev.icerock.moko.media

class FileMedia(
val name: String,
val path: String
val path: String,
val byteArray: ByteArray
Copy link
Member

Choose a reason for hiding this comment

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

store of readed file is not optimized.
you can save some additional data about reading of file to this obj, but not store ByteArray please - it can be large

@@ -4,7 +4,7 @@

package dev.icerock.moko.media

data class Media(
data class Media constructor(
Copy link
Member

Choose a reason for hiding this comment

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

why it changed?

if (callbackData !is CallbackData.Camera){
callbackData.callback.invoke(
Result.failure(
java.lang.IllegalStateException("Callback type should be Camera")
Copy link
Member

Choose a reason for hiding this comment

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

why java.lang?

Comment on lines +80 to +95
"application/pdf",
"application/octet-stream",
"application/doc",
"application/msword",
"application/ms-doc",
"application/vnd.ms-excel",
"application/vnd.ms-powerpoint",
"application/json",
"application/zip",
"text/plain",
"text/html",
"text/xml",
"audio/mpeg",
"application/vnd.openxmlformats-officedocument.wordprocessingml.document",
"application/vnd.openxmlformats-officedocument.presentationml.presentation",
"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"
Copy link
Member

Choose a reason for hiding this comment

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

maybe use something like **/*?

Comment on lines +52 to +55
val byteArray = requireContext()
.contentResolver
.openInputStream(uri)
?.readBytes() ?: return@registerForActivityResult
Copy link
Member

Choose a reason for hiding this comment

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

why we should read stream in this time? can we read it later?

val cursorRef = contentResolver
.query(uri, projection, null, null, null)
.query(uri, null, null, null, null)
Copy link
Member

Choose a reason for hiding this comment

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

why projection removed? we use only 2 columns at all


val cursorRef = contentResolver
.query(uri, projection, null, null, null)
.query(uri, null, null, null, null)
Copy link
Member

Choose a reason for hiding this comment

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

why projection removed? we use only 2 columns at all

Comment on lines +27 to +35
<provider
android:name="androidx.core.content.FileProvider"
android:authorities="${applicationId}.provider"
android:exported="false"
android:grantUriPermissions="true">
<meta-data
android:name="android.support.FILE_PROVIDER_PATHS"
android:resource="@xml/file_provider_paths" />
</provider>
Copy link
Member

Choose a reason for hiding this comment

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

for what purpose we should add provider? we not share own files outside of app

@Alex009 Alex009 added this to the 0.11.1 milestone Apr 18, 2024
Alexey Nesterov added 3 commits April 19, 2024 15:07
# Conflicts:
#	media/src/androidMain/kotlin/dev/icerock/moko/media/picker/MediaPickerControllerImpl.kt
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