I am new to Oppia-Android project, I would like to work on this issue, could you plz assign it to me? @adhiamboperes @MohitGupta121

I am new to Oppia-Android project, I would like to work on this issue, could you plz assign it to me? @adhiamboperes @MohitGupta121

Could you please give a brief description of how you would go about solving this.

This happens on tablets as well.

Thanks @adhiamboperes for tablet investigation.

Assign#5150

Assign#5150

Hi, in order to be assigned any issue, please give a brief description of how you would go about solving it.

Hey there!

I want to work on this project as well.

I tried to understand what's going on here, I failed:). It has to do something with margins (this function).

I had some non viable hypothesis. One interesting thing here is that when this bug happens this function I linked first is called for the 5th (last) element and only then for 0th, 1st...

Anyway, I came up with this solution. I don't know why it works, but the code becomes simpler and it works - the problem doesn't appear anymore, see the video. I checked it on Pixel 3 and Pixel C.

I understand it changes the layout, but I think it's not that noticeable. I also understand it's not a proper solution, but it works:) See the new layout for Pixel 3 and Pixel C attached.

WDYT about this approach?

Pixel3
Pixel3_landscape
Pixel C

Hi @nikitaevg -- thanks for this! Just from the PM side -- in the new layout, I think that the tiles in the right column in the Pixel 3 mocks are too close to the right edge of the screen. The margin should be consistent on the left and on the right. Is that achievable with your approach?

Also, I suggest showing what the equivalent mocks are for the current codebase so that the difference between the old/new ones can be seen more easily,

@adhiamboperes do you have any thoughts on the technical approach used here? Just as an FYI I found the relevant PR that introduced the changes (using the blame functionality in GitHub): see https://github.com/oppia/oppia-android/pull/2246/files#diff-9d65508457d46773757a6d2bb48b791b0dee5f9fb9e46c673a808c5de2294172R49 -- @nikitaevg maybe the comments and context there might be interesting as well.

Hey Sean!

Thanks for your comment! Yeah, it was my bad they were inconsistent on the left and right. I thought it was like this before. Sure, I don't want to break it:)

I found a real problem here. My new proposal is here, will create a proper PR soon.

The gist is these cards that become unaligned - they are being reused after returning to the main screen. Their margin is updated in the code, but the RecyclerView is not notified about those updates.

Check this video where you can see the bug is gone.

Hey Sean!

Thanks for your comment! Yeah, it was my bad they were inconsistent on the left and right. I thought it was like this before. Sure, I don't want to break it:)

I found a real problem here. My new proposal is here, will create a proper PR soon.

The gist is these cards that become unaligned - they are being reused after returning to the main screen. Their margin is updated in the code, but the RecyclerView is not notified about those updates.

Check this video where you can see the bug is gone.

Hi,
I think this approach is better and it is consistent with my own investigation into the root cause of the bug as well.

@nikitaevg proposed calling View.setLayoutParams() instead of View.requestLayout() in MarginBindingAdapters in #5160.

View.setLayoutParams() sets the layout parameters associated with this view. These supply parameters to the parent of this view specifying how it should be arranged.
View.requestLayout() should be called when something has changed which has invalidated the layout of this view. This will schedule a layout pass of the view tree. View.setLayoutParams() calls this method. At a glance, this does not handle the provided layout params, unlike the other method.

We need to test this code to prevent regressions in the UI, and the first step is to set up a test that does something very similar to the bug and see if we can repro the breakage in Robolectric, then we can step-debug to see what is causing the breakage, and set up a proper test for the code.

• This means setting up the test in HomeFragmentTest which has the layout under test.
• Observe the behavior when manually reproing the issue and try to turn it into a test. This acts like a debuggable environment for analyzing android.

Unable to reproduce the error. Am I doing something wrong?

Hi @Abinash6000, could you please try to repro this on a tablet device like a nexus 9?

Still unable to reproduce the error.

Hii, it would be of great help if anyone let's me know whether this issue has been solved yet or not. I would like to work on it

@AyeshaLoladia, please try to reproduce this issue on mobile and tablet emulators and then let us know so we can assign it to you!

@MohitGupta121 It's about a year since this issue was created why has nobody solved this issue, I just saw this bug and wanted to open an issue and found It already exists and it is a terrible sign for any app,
it would be better if you could make this impact medium from low, Thank you

@Vinay-Khanagavi Thanks for the note! I agree with you that it is medium-impact, and I have updated the label accordingly.

Hey @adhiamboperes , I’d like to start working on this issue, but I noticed PR #5160 . already seems to address it effectively.
Could you let me know if there were any specific concerns that prevented it from being merged?
This would help me understand how to proceed or make any needed improvements.
Thank you!

@TanishMoral11, there were some incomplete tests and the contributor abandoned the PR. The solution is otherwise acceptable

Thank you for clarifying, @adhiamboperes !

Would it be alright if I submit a new PR with the same changes and ensure that all the tests are completed?
I’d be happy to take it forward and address any additional requirements if needed.
Thank you!

Assigned this issue to you @TanishMoral11