Photos new #14

Rembremmerdeng · 2020-05-24T22:04:25Z

No description provided.

…ium.com/@tylerwalker/property-aware-mutablelivedata-with-kotlin-4fb5830ae730

… button behaviour in order to behave correctly if take photo button is not available

…ertyChanged is needed since only a resetting of the collection would be notififed automatically by PropertyAwareLiveData, adding an item does not invoke this

…ng in Android.Q and above

…oRecycler locally

…ms, fix creation of thumbnails for new versions of Android

…ew picture temporarily and save it when the permission has been granted

…ctoring

carstenhag · 2020-05-25T12:11:01Z

app/src/main/AndroidManifest.xml

-
+    <uses-feature android:name="android.hardware.camera"
+        android:required="false" />
+    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />


Müssen wir wirklich im externen storage speichern? Können wir das nicht app-intern tun? Note: Bisher hab ich noch nie Fotos aufgenommen innerhalb einer App, hab da also keine Erfahrung :D

I was debating about this as well. I came to the conclusion that in some cases it might be useful to have to photos taken with the app also on external storage. For example you can't finish the transmission (due to network connection) and you want to do it with weg-li website mass-upload later.

carstenhag · 2020-05-25T12:13:21Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

        setupCarTypeSpinner()
        setupViolationSpinner()
        durationText.setOnClickListener {
        }

        sendButton.setOnClickListener { mainViewModel.sendReport() }
+
+        val licenseObserver = Observer<Report> { newName ->
+            if(newName.license != "") {


Please reformat the file (use Android Studio's builtin way to do that). Also, you can use isEmpty or isNotEmpty here.

carstenhag · 2020-05-25T12:14:22Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+        mainViewModel.propertyAwareReport.observe(this, photosCountObserver)
+    }
+
+    fun convertPixelsToDp(px: Float, context: Context): Float {


Please move this out into an utils class or an extension class. Should not be declared in an activity.

carstenhag · 2020-05-25T12:14:29Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+            .displayMetrics.densityDpi.toFloat() / DisplayMetrics.DENSITY_DEFAULT)
+    }
+
+    private fun Float.convertDpToPixel(context: Context): Float {


carstenhag · 2020-05-25T12:16:24Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+
+
+    private fun setupPhotoRecyclerView(context: Context) {
+        val recyclerView = findViewById<RecyclerView>(R.id.photos_grid)


Please either use kotlin synthetics or the new View Binding (which afaik is now the recommended way to do this), see https://medium.com/androiddevelopers/use-view-binding-to-replace-findviewbyid-c83942471fc

I would just use "photos_grid" (should be renamed to "photosRecyclerView"), kotlin will do the rest.

carstenhag · 2020-05-25T12:32:43Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+        val resolver: ContentResolver = contentResolver
+        val contentValues = ContentValues().apply {
+            put(MediaStore.MediaColumns.DISPLAY_NAME, image_name)
+            put(MediaStore.MediaColumns.MIME_TYPE, "image/jpeg")


Please look up whether there is a constant for this MIME type and use it if possible.

carstenhag · 2020-05-25T12:33:02Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+            }
+        }
+        val imageUri: Uri =
+            resolver.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, contentValues)!!


Please avoid all the !! here.

carstenhag · 2020-05-25T12:33:50Z

app/src/main/java/com/github/weg_li_android/PhotoRecyclerViewAdapter.kt

+    private val mInflater: LayoutInflater = LayoutInflater.from(context)
+    private var mClickListener: ItemClickListener? = null
+    var deletePhotoSet: ObservableArrayList<Int> = ObservableArrayList()
+    /*init {


Please clear this up

carstenhag · 2020-05-25T12:34:12Z

app/src/main/java/com/github/weg_li_android/PhotoRecyclerViewAdapter.kt

+        )
+    }*/
+
+    @NonNull


Nullable annotations are not needed on Kotlin-only code :)

This would also not be needed if we had both java + kotlin code. This is only useful to add on Java code.

carstenhag · 2020-05-25T12:35:11Z

app/src/main/java/com/github/weg_li_android/PhotoRecyclerViewAdapter.kt

+
+    }
+*/
+    @Throws(FileNotFoundException::class, IOException::class)


Don't use checked exceptions like this on Kotlin code.

carstenhag · 2020-05-25T12:36:39Z

app/src/main/java/com/github/weg_li_android/PhotoRecyclerViewAdapter.kt

+            val thumbnailSize = 120f.convertDpToPixel(context).toInt()
+            val thumbnail: Bitmap? =
+                uri?.let {
+                    context.contentResolver.loadThumbnail(


This is an API29 method, but our min sdk level is 19 or 21. It will always crash. Android Studio should also give you an error for this.

leowroth · 2020-05-25T15:37:05Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

    private lateinit var mainViewModel: MainViewModel
+    private val pickImage = 1
+    private val takeImage = 2
+    private var mActionMode: ActionMode? = null


Please let's not use the Hungarian notation, Android Studio tells us the type. Please change "mActionMode" to "actionMode"

leowroth · 2020-05-25T15:38:49Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+
+        val licenseObserver = Observer<Report> { newName ->
+            if(newName.license != "") {
+                status_image.setImageResource(R.drawable.ic_baseline_check_circle_24)


In Kotlin it's better to name it "statusImage" instead of "status_image", since we're using these names just like here in code.

leowroth · 2020-05-25T15:40:21Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+
+        val pm: PackageManager = context.packageManager
+        if (!pm.hasSystemFeature(PackageManager.FEATURE_CAMERA)) {
+            take_picture_button.visibility = View.GONE


Rename to "takePictureButton"

leowroth · 2020-05-25T15:41:47Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+
+        val photosCountObserver = Observer<Report> { newName ->
+            if(newName.violationPhotos.size >= 2) {
+                photos_status_image.setImageResource(R.drawable.ic_baseline_check_circle_24)


rename to "photosStatusImage"

You can import "R.drawable", this will shorten every use of drawables

leowroth · 2020-05-25T15:51:39Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+            override fun onGlobalLayout() {
+                recyclerView.viewTreeObserver.removeOnGlobalLayoutListener(this)
+                //layoutManager.setColumnWidth(convertPixelsToDp(recyclerView.width.toFloat(), context).toInt())
+                Timber.e(convertPixelsToDp(recyclerView.width.toFloat(), context).toString()) //height is ready


Did you use this to debug? Should be removed, once this part is done.

leowroth · 2020-05-25T15:52:02Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+                Timber.e(convertPixelsToDp(recyclerView.width.toFloat(), context).toString()) //height is ready
+            }
+        })
+


Remove extra empty lines

leowroth · 2020-05-25T15:52:51Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+        photoAdapter.setClickListener(this)
+        recyclerView.adapter = photoAdapter
+
+        take_picture_button.setOnTouchListener { v, event ->


Rename to "takePictureButton"

leowroth · 2020-05-25T15:53:05Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+            false
+        }
+
+        add_picture_button.setOnTouchListener { v, event ->


rename to addPictureButton

leowroth · 2020-05-25T15:54:31Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+    }
+
+    override fun onItemLongClick(view: View?, position: Int) {
+        Timber.e("You looong clicked number %s", position.toString())


Did you use this for debugging? Should be removed, once feature is done.

leowroth · 2020-05-25T15:54:47Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+
+    override fun onItemClick(view: View?, position: Int) {
+        if(mActionMode != null) {
+            Timber.e("You clicked number %s", position.toString())


Did you use this for debugging? Should be removed, once feature is done.

leowroth · 2020-05-25T15:55:46Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+            photoAdapter.notifyItemChanged(position)
+    }
+
+    private val mActionModeCallback: ActionMode.Callback =


Don't use Hungarian notation, rename to "actionModeCallback"

leowroth · 2020-05-25T15:56:18Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+                item: MenuItem
+            ): Boolean {
+                return when (item.itemId) {
+                    R.id.delete_button -> {


rename to "deleteButton"

leowroth · 2020-05-25T15:56:42Z

app/src/main/java/com/github/weg_li_android/MainActivity.kt

+            ): Boolean {
+                return when (item.itemId) {
+                    R.id.delete_button -> {
+                        Timber.e("I'm here. Option1 selected, i will remove "+photoAdapter.deletePhotoSet.sorted().toString())


Used for debugging? Remove once feature is done

Rembremmerdeng added 12 commits April 26, 2020 15:41

Add PropertyAwareMutableLiveData pattern for binding from https://med…

3d58881

…ium.com/@tylerwalker/property-aware-mutablelivedata-with-kotlin-4fb5830ae730

Add photo card

6c469d3

Disable button for taking photo if no camera is available, change add…

d878e09

… button behaviour in order to behave correctly if take photo button is not available

Add binding for violation photos to status image, a manual notifyProp…

2baf48e

…ertyChanged is needed since only a resetting of the collection would be notififed automatically by PropertyAwareLiveData, adding an item does not invoke this

Save pictures to external default image directory, for now only worki…

045f473

…ng in Android.Q and above

Store Uris only of taken or added photos, generate thumbnails in Phot…

3293d56

…oRecycler locally

Fix grid rendering in photos tab, add possibility to check single ite…

c10d55e

…ms, fix creation of thumbnails for new versions of Android

Make Photo Card feature complete, add delete menu and functionality

c0d95d5

Refactoring, remove dead stubs

c9bd911

Add possibility to add photos if only adding one is supported

4d1c990

If permission is requested for the first time, we need to store the n…

0ae94f1

…ew picture temporarily and save it when the permission has been granted

Move viewmodel to adapter in order to directly notify on change, refa…

4cfe8e9

…ctoring

Rembremmerdeng linked an issue May 24, 2020 that may be closed by this pull request

Map Card #5

Open

Rembremmerdeng removed a link to an issue May 24, 2020

Map Card #5

Open

Rembremmerdeng linked an issue May 24, 2020 that may be closed by this pull request

Photos Card #7

Open

carstenhag suggested changes May 25, 2020

View reviewed changes

carstenhag reviewed May 25, 2020

View reviewed changes

leowroth reviewed May 25, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Photos new #14

Photos new #14

Rembremmerdeng commented May 24, 2020

carstenhag May 25, 2020

Rembremmerdeng May 26, 2020

carstenhag May 25, 2020

carstenhag May 25, 2020

carstenhag May 25, 2020

carstenhag May 25, 2020

leowroth May 25, 2020

carstenhag May 25, 2020

carstenhag May 25, 2020

carstenhag May 25, 2020

carstenhag May 25, 2020

carstenhag May 25, 2020

carstenhag May 25, 2020

carstenhag May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020

leowroth May 25, 2020



		private fun setupPhotoRecyclerView(context: Context) {
		val recyclerView = findViewById<RecyclerView>(R.id.photos_grid)

+                      )
+                  }*/
+                  @NonNull

Photos new #14

Are you sure you want to change the base?

Photos new #14

Conversation

Rembremmerdeng commented May 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment