Carousel numViewsToShowOnScreen and config changes

[mradzinski]

Hey @elihart, I've been trying to set numViewsToShowOnScreen on a Carousel to a percentage of the display width (which gets updated upon config changes). To achieve this, what I'm doing is quite simple:

numViewsToShowOnScreen((17 * screenWidthPx / 100) / 100) // Where 17 is the percentage visible

This works well upon rendering the first time, but it breaks as soon as I rotate the device... and if I scroll through the carousel it seems to fix itself (probably due to recycling of the modes). I'm attaching a short GIF of what's happening.

Any idea of what might be happening? I've tried invalidating the RecyclerView and the Carousel itself (during onBind), also assigning Random id's to force Epoxy to rebuild the models, but none of this seem to solve the issue.

[mradzinski]

I ended up solving this through using WRAP_WIDTH_WRAP_HEIGHT on the @Model, setting a padding on the CarouselModel_, and stopped using numViewsToShowOnScreen.

Still not sure if numViewsToShowOnScreen should behave this way though. If this behavior is to be expected then feel free close the issue @elihart.

[elihart]

Thanks for the video - are you handling the configuration change yourself, or is the activity being recreated on rotation? if it is being recreated I don't see how the views would remember their old width

[mradzinski]

@elihart I don't use activities, but the view containing the RV re-inflates and has a different instance, so I think it would count as "being recreated". I'm quite sure of this given the instances of the RV on config changes are different too (hash is different).

I think it has something to do with recycling/rendering. The moment I scroll and "force" the items to recycle the issue is gone.

[elihart]

In a situation where rotation causes an activity recreation this should work as expected, and in general numViewsToShowOnScreen should update across recreation.

I am not familiar with how you are using it without an activity though and don't know how that affects view recreation. if you'd like to look into it you can see how Carousel#numViewsToShowOnScreen is being called, since that has the view resizing logic

[mradzinski]

Agreed. I'll check the class and will try finding the reason behind this based on that. I'll let you know if I find anything reproducible using activities or fragments. In the meantime let's close this issue to keep the backlog clean, I'll open a new one if needed.

Thanks again for your help Eli and cheers!

[AkshayChordiya]

I faced the same issue even if I was setting a constant numViewsToShowOnScreen. It looked weird on config changes. Hence I created a custom Carousel and override createLayoutManager which solved the problem with configuration and gave exactly what I wanted:

override fun createLayoutManager() = GridLayoutManager(
      context, NUMBER_OF_VIEW_ON_SCREEN, GridLayoutManager.VERTICAL, false
)

[andybdahl]

I deep dived into this a bit as I have experienced problem multiple times before. The culprit is the Carousel.getTotalWidthPx function, as this returns the wrong value after a configuration change. At first the totalWidthPx returns the measured width of the carousel as it hasn't been laid out yet, which is the correct totalWidth for the carousel. After a configuration change, the carousel is already laid out before the change, and so the getTotalWidthPx returns the carousels actual width, which is "wrong" as it was the width before the configuration change, so this could be fixed by checking if the screen was rotated since the view was laid out, and thus returning a measured value instead, as the view hasn't been laid out in the current configuration yet.

[andybdahl]

After my deep dive and look at other issues, I found that that issue started happening with the merge of #915 . The cause of the issue likely is the same cause of #1073 as well, although this would need to be tested after a fix has been made to correct the issue.

[elihart]

Thanks for the investigation @andybdahl , that seems to make sense. It does seem a little difficult to detect when the measured width/height is not accurate though. Some possibilities:

• figure out how to properly detect a configuration change
• add an option to the carousel to not use measured dimensions

[andybdahl]

@EliHart We fixed it locally in our app already, I'll be putting up a merge request when I get the time for it soon.

I fixed it by checking whether or not the carousel has been rendered dirty after a configuration change, so it's rather simple.

On Thu, 11 Feb 2021, 17.10 Eli Hart, notifications@github.com wrote:

Thanks for the investigation @andybdahl https://github.com/andybdahl , that seems to make sense. It does seem a little difficult to detect when the measured width/height is not accurate though. Some possibilities:

• figure out how to properly detect a configuration change
• add an option to the carousel to not use measured dimensions

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/airbnb/epoxy/issues/682#issuecomment-777607472, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHDZZORFK2YCREPZLTEQL2TS6P6NRANCNFSM4GW42Z5A .

[elihart]

sounds good, would be great to have your contribution when you have time 🙏