Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Fix camera settings item blinking (EXPOSUREAPP-6337) #2801

Merged
merged 8 commits into from
Apr 12, 2021

Conversation

mtwalli
Copy link
Contributor

@mtwalli mtwalli commented Apr 11, 2021

  • With the introduction of intervalFlow in CheckIns screen causes a refresh every second, when Camera item is displayed it had blinking effect to avoid that this PR create only one item lazily
  • Fix typo

@mtwalli mtwalli added bug Something isn't working maintainers Tag pull requests created by maintainers labels Apr 11, 2021
@mtwalli mtwalli added this to the 2.0.0 milestone Apr 11, 2021
@mtwalli mtwalli requested a review from a team April 11, 2021 17:24
@mtwalli mtwalli changed the title Fix camera settings item blinking (DEV Fix camera settings item blinking (DEV) Apr 11, 2021
@d4rken d4rken self-assigned this Apr 12, 2021
@d4rken d4rken self-requested a review April 12, 2021 06:33
d4rken
d4rken previously approved these changes Apr 12, 2021
Copy link
Member

@d4rken d4rken left a comment

Choose a reason for hiding this comment

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

Works for me, as the CameraItem was only instantiated once in init. The other solution would be to use HasPayloadDiffer.

@mtwalli
Copy link
Contributor Author

mtwalli commented Apr 12, 2021

Works for me, as the CameraItem was only instantiated once in init. The other solution would be to use HasPayloadDiffer.

I did try it as :

 override fun diffPayload(old: Any, new: Any): Any? = null

did not work for me , not sure if this how it should be ?

@mtwalli mtwalli changed the title Fix camera settings item blinking (DEV) Fix camera settings item blinking (EXPOSUREAPP-6337) Apr 12, 2021
@d4rken
Copy link
Member

d4rken commented Apr 12, 2021

did not work for me , not sure if this how it should be ?

You need to return a non-null value.

class AsyncDiffer<T : HasStableId>(
adapter: RecyclerView.Adapter<*>,
compareItem: (T, T) -> Boolean = { i1, i2 -> i1.stableId == i2.stableId },
compareItemContent: (T, T) -> Boolean = { i1, i2 -> i1 == i2 },
determinePayload: (T, T) -> Any? = { i1, i2 ->
when {
i1 is HasPayloadDiffer && i1::class == i2::class -> i1.diffPayload(i1, i2)
else -> null
}
}
) {
private val callback = object : DiffUtil.ItemCallback<T>() {
override fun areItemsTheSame(oldItem: T, newItem: T): Boolean = compareItem(oldItem, newItem)
override fun areContentsTheSame(oldItem: T, newItem: T): Boolean = compareItemContent(oldItem, newItem)
override fun getChangePayload(oldItem: T, newItem: T): Any? = determinePayload(oldItem, newItem)
}

Other cards we have use this pattern:

) : TestResultItem, HasPayloadDiffer {
override fun diffPayload(old: Any, new: Any): Any? = if (old::class == new::class) new else null
}

then

override val onBindData: HomeSubmissionStatusCardErrorBinding.(
item: Item,
payloads: List<Any>
) -> Unit = { item, payloads ->
val curItem = payloads.filterIsInstance<Item>().singleOrNull() ?: item
itemView.setOnClickListener { curItem.onDeleteTest(item) }
showTestAction.setOnClickListener { itemView.performClick() }
}

The idea being that we want to replace any stale on-click listeners that may reference something that no longer exists due to recreation on lifecycle events. Not sure if it's necessary in this case, but using the payload mechanism tells the RecyclerView that we will take care of any layout that is necessary, so you may be able to just return Any() as payload diff.

@mtwalli
Copy link
Contributor Author

mtwalli commented Apr 12, 2021

did not work for me , not sure if this how it should be ?

You need to return a non-null value.

class AsyncDiffer<T : HasStableId>(
adapter: RecyclerView.Adapter<*>,
compareItem: (T, T) -> Boolean = { i1, i2 -> i1.stableId == i2.stableId },
compareItemContent: (T, T) -> Boolean = { i1, i2 -> i1 == i2 },
determinePayload: (T, T) -> Any? = { i1, i2 ->
when {
i1 is HasPayloadDiffer && i1::class == i2::class -> i1.diffPayload(i1, i2)
else -> null
}
}
) {
private val callback = object : DiffUtil.ItemCallback<T>() {
override fun areItemsTheSame(oldItem: T, newItem: T): Boolean = compareItem(oldItem, newItem)
override fun areContentsTheSame(oldItem: T, newItem: T): Boolean = compareItemContent(oldItem, newItem)
override fun getChangePayload(oldItem: T, newItem: T): Any? = determinePayload(oldItem, newItem)
}

Other cards we have use this pattern:

) : TestResultItem, HasPayloadDiffer {
override fun diffPayload(old: Any, new: Any): Any? = if (old::class == new::class) new else null
}

then

override val onBindData: HomeSubmissionStatusCardErrorBinding.(
item: Item,
payloads: List<Any>
) -> Unit = { item, payloads ->
val curItem = payloads.filterIsInstance<Item>().singleOrNull() ?: item
itemView.setOnClickListener { curItem.onDeleteTest(item) }
showTestAction.setOnClickListener { itemView.performClick() }
}

The idea being that we want to replace any stale on-click listeners that may reference something that no longer exists due to recreation on lifecycle events. Not sure if it's necessary in this case, but using the payload mechanism tells the RecyclerView that we will take care of any layout that is necessary, so you may be able to just return Any() as payload diff.

In this case , current solution is sufficient, item does not change and click listener does not any data

@harambasicluka harambasicluka self-assigned this Apr 12, 2021
@sonarcloud
Copy link

sonarcloud bot commented Apr 12, 2021

Kudos, SonarCloud Quality Gate passed!

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

50.0% 50.0% Coverage
0.0% 0.0% Duplication

@harambasicluka
Copy link
Contributor

harambasicluka commented Apr 12, 2021

@mtwalli how do I get this card? Tried it on an Xiamoi Mi (Android 9) and Oneplus 6t (Android 10) by clicking deny in the scanning screen.

@harambasicluka
Copy link
Contributor

@mtwalli how do I get this card? Tried it on an Xiamoi Mi (Android 9) and Oneplus 6t (Android 10) by clicking deny in the scanning screen.

Ok found it, you have deny once and then the second time you get asked if you don't want to be asked again.

Works on both devices :)

@mtwalli mtwalli merged commit 8faf968 into release/2.0.x Apr 12, 2021
@mtwalli mtwalli deleted the fix/camera_item_blinking branch April 12, 2021 14:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working maintainers Tag pull requests created by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants