Skip to content

Commit 1c6ebaf

Browse files
feat: show the proper message in the custom picker when all images are either uploaded or marked. (commons-app#6095)
* feat: show the message "Congratulations, all pictures in this album have been either uploaded or marked as not for upload." in the custom picker when all images are either uploaded or marked. * refactor: fix indentation in the code * refactor: replace LiveData with StateFlow * fix: fixed the bug that was causing one ImageFragment testcase to fail. * fix: fixed the Congratulation message being shown for a brief moment while switching off the switch * refactor: move hardcoded string to strings.xml. * docs: add comment to clarify visibility logic in ImageFragment --------- Co-authored-by: Nicolas Raoul <[email protected]>
1 parent 23e1f01 commit 1c6ebaf

File tree

4 files changed

+81
-3
lines changed

4 files changed

+81
-3
lines changed

app/src/main/java/fr/free/nrw/commons/customselector/ui/adapter/ImageAdapter.kt

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import kotlinx.coroutines.CoroutineScope
2424
import kotlinx.coroutines.Dispatchers
2525
import kotlinx.coroutines.MainScope
2626
import kotlinx.coroutines.cancel
27+
import kotlinx.coroutines.flow.MutableStateFlow
2728
import kotlinx.coroutines.launch
2829
import java.util.TreeMap
2930
import kotlin.collections.ArrayList
@@ -103,6 +104,18 @@ class ImageAdapter(
103104
*/
104105
private var imagePositionAsPerIncreasingOrder = 0
105106

107+
/**
108+
* Stores the number of images currently visible on the screen
109+
*/
110+
private val _currentImagesCount = MutableStateFlow(0)
111+
val currentImagesCount = _currentImagesCount
112+
113+
/**
114+
* Stores whether images are being loaded or not
115+
*/
116+
private val _isLoadingImages = MutableStateFlow(false)
117+
val isLoadingImages = _isLoadingImages
118+
106119
/**
107120
* Coroutine Dispatchers and Scope.
108121
*/
@@ -184,8 +197,12 @@ class ImageAdapter(
184197
// If the position is not already visited, that means the position is new then
185198
// finds the next actionable image position from all images
186199
if (!alreadyAddedPositions.contains(position)) {
187-
processThumbnailForActionedImage(holder, position, uploadingContributionList)
188-
200+
processThumbnailForActionedImage(
201+
holder,
202+
position,
203+
uploadingContributionList
204+
)
205+
_isLoadingImages.value = false
189206
// If the position is already visited, that means the image is already present
190207
// inside map, so it will fetch the image from the map and load in the holder
191208
} else {
@@ -231,6 +248,7 @@ class ImageAdapter(
231248
position: Int,
232249
uploadingContributionList: List<Contribution>,
233250
) {
251+
_isLoadingImages.value = true
234252
val next =
235253
imageLoader.nextActionableImage(
236254
allImages,
@@ -252,6 +270,7 @@ class ImageAdapter(
252270
actionableImagesMap[next] = allImages[next]
253271
alreadyAddedPositions.add(imagePositionAsPerIncreasingOrder)
254272
imagePositionAsPerIncreasingOrder++
273+
_currentImagesCount.value = imagePositionAsPerIncreasingOrder
255274
Glide
256275
.with(holder.image)
257276
.load(allImages[next].uri)
@@ -267,6 +286,7 @@ class ImageAdapter(
267286
reachedEndOfFolder = true
268287
notifyItemRemoved(position)
269288
}
289+
_isLoadingImages.value = false
270290
}
271291

272292
/**
@@ -372,6 +392,7 @@ class ImageAdapter(
372392
emptyMap: TreeMap<Int, Image>,
373393
uploadedImages: List<Contribution> = ArrayList(),
374394
) {
395+
_isLoadingImages.value = true
375396
allImages = fixedImages
376397
val oldImageList: ArrayList<Image> = images
377398
val newImageList: ArrayList<Image> = ArrayList(newImages)
@@ -382,6 +403,7 @@ class ImageAdapter(
382403
reachedEndOfFolder = false
383404
selectedImages = ArrayList()
384405
imagePositionAsPerIncreasingOrder = 0
406+
_currentImagesCount.value = imagePositionAsPerIncreasingOrder
385407
val diffResult =
386408
DiffUtil.calculateDiff(
387409
ImagesDiffCallback(oldImageList, newImageList),
@@ -441,6 +463,7 @@ class ImageAdapter(
441463
val entry = iterator.next()
442464
if (entry.value == image) {
443465
imagePositionAsPerIncreasingOrder -= 1
466+
_currentImagesCount.value = imagePositionAsPerIncreasingOrder
444467
iterator.remove()
445468
alreadyAddedPositions.removeAt(alreadyAddedPositions.size - 1)
446469
notifyItemRemoved(index)

app/src/main/java/fr/free/nrw/commons/customselector/ui/selector/ImageFragment.kt

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,12 @@ import android.widget.ProgressBar
1212
import android.widget.Switch
1313
import androidx.appcompat.app.AlertDialog
1414
import androidx.constraintlayout.widget.ConstraintLayout
15+
import androidx.core.view.isVisible
16+
import androidx.lifecycle.Lifecycle
1517
import androidx.lifecycle.Observer
1618
import androidx.lifecycle.ViewModelProvider
19+
import androidx.lifecycle.lifecycleScope
20+
import androidx.lifecycle.repeatOnLifecycle
1721
import androidx.recyclerview.widget.GridLayoutManager
1822
import androidx.recyclerview.widget.RecyclerView
1923
import fr.free.nrw.commons.contributions.Contribution
@@ -38,6 +42,10 @@ import fr.free.nrw.commons.theme.BaseActivity
3842
import fr.free.nrw.commons.upload.FileProcessor
3943
import fr.free.nrw.commons.upload.FileUtilsWrapper
4044
import io.reactivex.schedulers.Schedulers
45+
import kotlinx.coroutines.flow.MutableStateFlow
46+
import kotlinx.coroutines.flow.asStateFlow
47+
import kotlinx.coroutines.flow.combine
48+
import kotlinx.coroutines.launch
4149
import java.util.TreeMap
4250
import javax.inject.Inject
4351
import kotlin.collections.ArrayList
@@ -80,6 +88,12 @@ class ImageFragment :
8088
*/
8189
var allImages: ArrayList<Image> = ArrayList()
8290

91+
/**
92+
* Keeps track of switch state
93+
*/
94+
private val _switchState = MutableStateFlow(false)
95+
val switchState = _switchState.asStateFlow()
96+
8397
/**
8498
* View model Factory.
8599
*/
@@ -214,7 +228,11 @@ class ImageFragment :
214228

215229
switch = binding?.switchWidget
216230
switch?.visibility = View.VISIBLE
217-
switch?.setOnCheckedChangeListener { _, isChecked -> onChangeSwitchState(isChecked) }
231+
_switchState.value = switch?.isChecked ?: false
232+
switch?.setOnCheckedChangeListener { _, isChecked ->
233+
onChangeSwitchState(isChecked)
234+
_switchState.value = isChecked
235+
}
218236
selectorRV = binding?.selectorRv
219237
loader = binding?.loader
220238
progressLayout = binding?.progressLayout
@@ -234,6 +252,28 @@ class ImageFragment :
234252
return binding?.root
235253
}
236254

255+
/**
256+
* onViewCreated
257+
* Updates empty text view visibility based on image count, switch state, and loading status.
258+
*/
259+
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
260+
super.onViewCreated(view, savedInstanceState)
261+
viewLifecycleOwner.lifecycleScope.launch {
262+
repeatOnLifecycle(Lifecycle.State.STARTED) {
263+
combine(
264+
imageAdapter.currentImagesCount,
265+
switchState,
266+
imageAdapter.isLoadingImages
267+
) { imageCount, isChecked, isLoadingImages ->
268+
Triple(imageCount, isChecked, isLoadingImages)
269+
}.collect { (imageCount, isChecked, isLoadingImages) ->
270+
binding?.allImagesUploadedOrMarked?.isVisible =
271+
!isLoadingImages && !isChecked && imageCount == 0 && (switch?.isVisible == true)
272+
}
273+
}
274+
}
275+
}
276+
237277
private fun onChangeSwitchState(checked: Boolean) {
238278
if (checked) {
239279
showAlreadyActionedImages = true

app/src/main/res/layout/fragment_custom_selector.xml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,20 @@
4949
app:layout_constraintStart_toStartOf="parent"
5050
app:layout_constraintTop_toTopOf="parent" />
5151

52+
<TextView
53+
android:id="@+id/all_images_uploaded_or_marked"
54+
android:layout_width="wrap_content"
55+
android:layout_height="wrap_content"
56+
android:visibility="gone"
57+
android:textSize="16sp"
58+
android:padding="@dimen/standard_gap"
59+
android:textColor="@color/text_color_selector"
60+
android:text="@string/congratulations_all_pictures_in_this_album_have_been_either_uploaded_or_marked_as_not_for_upload"
61+
app:layout_constraintTop_toTopOf="parent"
62+
app:layout_constraintBottom_toBottomOf="parent"
63+
app:layout_constraintEnd_toEndOf="parent"
64+
app:layout_constraintStart_toStartOf="parent"
65+
/>
5266

5367
<ProgressBar
5468
android:id="@+id/loader"

app/src/main/res/values/strings.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,5 +867,6 @@ Upload your first media by tapping on the add button.</string>
867867
<string name="account_vanish_request_confirm"><![CDATA[Vanishing is a <b>last resort</b> and should <b>only be used when you wish to stop editing forever</b> and also to hide as many of your past associations as possible.<br/><br/>Account deletion on Wikimedia Commons is done by changing your account name to make it so others cannot recognize your contributions in a process called account vanishing. <b>Vanishing does not guarantee complete anonymity or remove contributions to the projects</b>.]]></string>
868868
<string name="caption">Caption</string>
869869
<string name="caption_copied_to_clipboard">Caption copied to clipboard</string>
870+
<string name="congratulations_all_pictures_in_this_album_have_been_either_uploaded_or_marked_as_not_for_upload">Congratulations, all pictures in this album have been either uploaded or marked as not for upload.</string>
870871

871872
</resources>

0 commit comments

Comments
 (0)