search

airbnb / epoxy

Epoxy is an Android library for building complex screens in a RecyclerView
https://goo.gl/eIK82p
Apache License 2.0
8.51k stars 728 forks source link

Some VisibilityStates should not be emitted after INVISIBLE #1343

Open shinwan2 opened 1 year ago

shinwan2 commented 1 year ago

Description

When the models are rebuilt, below snippets https://github.com/airbnb/epoxy/blob/109d434d796da68f8599bf68ccc13cf833717a1f/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyVisibilityTracker.kt#L331-L340 may emit events in below order

VisibilityState.INVISIBLE
VisibilityState.PARTIAL_IMPRESSION_INVISIBLE
VisibilityState.UNFOCUSED_VISIBLE

although detachEvent = true.

The screenshot below shows the call stack when the view is detached after models are rebuilt.

BugEpoxyVisibility

Suggestion

Assuming that VisiblityState.VISIBLE is the only initial state and VisibilityState.INVISIBLE the terminal state, I find the above sequence unexpected. Shouldn't they be

VisibilityState.VISIBLE
// Some other visible states
// The user refreshes the feed 
VisibilityState.UNFOCUSED_VISIBLE
VisibilityState.PARTIAL_IMPRESSION_INVISIBLE
VisibilityState.INVISIBLE
// No other VisibilityStates

or with smaller code changes maybe we can just

VisibilityState.VISIBLE
// Some other visible states
// The user refreshes the feed 
VisibilityState.INVISIBLE
// No other VisibilityStates

Environment

Epoxy version 4.6.4