Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace current recyclerview implementation with BindableAdapter usage #632

Closed
12 of 13 tasks
rt4914 opened this issue Jan 30, 2020 · 40 comments · Fixed by #5077
Closed
12 of 13 tasks

Replace current recyclerview implementation with BindableAdapter usage #632

rt4914 opened this issue Jan 30, 2020 · 40 comments · Fixed by #5077
Assignees
@masclot
Labels
bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@rt4914
Copy link
Contributor

rt4914 commented Jan 30, 2020 •
edited by adhiamboperes
Loading

Currently the entire project has two types of RecyclerView implementation:

  1. Recyclerview using BindableAdapter (OngoingStoryListActivity, CompletedStoryListActivity, etc.) uses it)

  2. Recyclerview with the RecyclerViewAdapter which inflates multiple layouts.

We need to change the implementation such that all RecyclerViewAdapter to use BindableAdapter (i.e. approach similar to OngoingStoryListActivity, CompletedStoryListActivity).

Checklist

  • ContinuePlaying (assigned to @veena14cs )
  • HomeFragment (assigned to @jcqli)
  • StoryActivity (Already done)
  • TopicReview/Revision (Assigned to @Sarthak2601 )
  • TopicPlay/Lessons
  • TopicTrain/Practice (assigned to @Luffy18346 )
  • HelpActivity (Assigned to @SayantanBanerjee16 )
  • HintsAndSolutionAdapter (Assigned to @anandwana001 )
  • OngoingListAdapter (open)
  • TopicListAdapter (assigned to @jcqli)
  • LanguageSelectionAdapter (assigned to @FareesHussain)
  • StorySummaryAdapter (open)
  • PromotedStoryListAdapter (assigned to @masclot )
@veena14cs
Copy link
Contributor

@rt4914 I will be taking up Continue Playing task in this list.

@veena14cs
Copy link
Contributor

@rt4914 StoryActivity already has implementation of Bindable adapter.

veena14cs added a commit that referenced this issue Feb 10, 2020
@veena14cs
Fix part #632: Replace current recyclerview implementation with Binda…
5484b19 
…bleAdapter usage. (#641)

* working on topic practice.

* reverted

* updated implementation

* Update ContinuePlayingFragmentPresenter.kt

* Update ContinuePlayingFragmentPresenter.kt

* Update ContinuePlayingItemViewModel.kt

* Delete OngoingListAdapter.kt

* nit change

* Update ContinuePlayViewModel.kt

* fixed nit
PrarabdhGarg pushed a commit to PrarabdhGarg/oppia-android that referenced this issue Feb 12, 2020
@veena14cs @PrarabdhGarg
Fix part oppia#632: Replace current recyclerview implementation with …
d9d4c8a 
…BindableAdapter usage. (oppia#641)

* working on topic practice.

* reverted

* updated implementation

* Update ContinuePlayingFragmentPresenter.kt

* Update ContinuePlayingFragmentPresenter.kt

* Update ContinuePlayingItemViewModel.kt

* Delete OngoingListAdapter.kt

* nit change

* Update ContinuePlayViewModel.kt

* fixed nit
rt4914 pushed a commit that referenced this issue Feb 13, 2020
@PrarabdhGarg @Luffy18346 @veena14cs
Fix #590: Full UI Switch Profile Option (#651)
018f06f 
* Fix #572: Keyboard visible by default in Admin Pin (#573)

* By default keyboard visibility issue in Admin Pin is resolved

* Created a test case to check about Keyboard visibility by default in Admin Pin.

* Test is modified with the suggested changes

* Fix #577:  Display profile name on navigation drawer. (#578)

* display profile name in navigation drawer

* Updated code implementation

* adding tests

* added testcases

* Update NavigationDrawerFragmentPresenter.kt

* updated nit changes.

* Fix part #44: Full UI profile pin/password screen. (#597)

* done ui corrections

* fixed issues.

* Update pin_password_activity.xml

* updated color

* updated icon

* updated pinview shadow

* updated icon color

* changed shadow

* updated mock color

* fix

* Update pin_password_activity.xml

* updated nit change.

* Update pin_password_activity.xml

* removed fixed width

* fixed dimen file changes

* Update dimens.xml

* Fix part #632: Replace current recyclerview implementation with BindableAdapter usage. (#641)

* working on topic practice.

* reverted

* updated implementation

* Update ContinuePlayingFragmentPresenter.kt

* Update ContinuePlayingFragmentPresenter.kt

* Update ContinuePlayingItemViewModel.kt

* Delete OngoingListAdapter.kt

* nit change

* Update ContinuePlayViewModel.kt

* fixed nit

* Add UI and finctionality for Switch Profile

* Add UI Tests for switch profile option in navigation drawer

* Add UI Tests for switch profile option in navigation drawer

* Minor styling changes

Co-authored-by: Akash Koradia <[email protected]>
Co-authored-by: Veena <[email protected]>
@PrarabdhGarg PrarabdhGarg mentioned this issue Mar 23, 2020
Closed
8 tasks
@NullByte08
Copy link
Contributor

@rt4914 Can I work on the HomeFragment task? I will replace both TopicListAdapter and PromotedStoryListAdapter.

@rt4914
Copy link
Contributor Author

rt4914 commented Apr 7, 2020 •
edited
Loading

@rt4914 Can I work on the HomeFragment task? I will replace both TopicListAdapter and PromotedStoryListAdapter.

@NullByte08 I would suggest you to work on simpler one first as HomeFragment is the hardest one. Still its completely fine if you work on that.

@Sarthak2601
Copy link
Contributor

@rt4914 Can I work on TopicReview/Revision task ?

@rt4914
Copy link
Contributor Author

rt4914 commented Apr 7, 2020

@rt4914 Can I work on TopicReview/Revision task ?

Sure.

@NullByte08
Copy link
Contributor

@rt4914 Can I work on FAQMain task then?

@SayantanBanerjee16
Copy link
Contributor

SayantanBanerjee16 commented Apr 7, 2020 •
edited
Loading

@rt4914 I think HelpActivity should also come in this list. Can you please add it and assign me HelpActivity?

@rt4914
Copy link
Contributor Author

rt4914 commented Apr 7, 2020

@SayantanBanerjee16 @NullByte08
The FAQMain is actually HelpActivity only. Now considering that @SayantanBanerjee16 has already worked on HelpActivity it would be nice to assign it to him. And also, as @NullByte08 had mentioned earlier regarding HomeActivity but I do think it is hard also, but as none of the easy one's are left now @NullByte08 can work on HomeActivity .

These arrangements have been done keep in mind the PRs that you have worked on in past.

@NullByte08
Copy link
Contributor

@rt4914 Alright sure.

@abhinavraj23
Copy link
Contributor

@rt4914 can I work on TopicPlay/Lessons?

@rt4914
Copy link
Contributor Author

rt4914 commented Apr 7, 2020

@abhinavraj23 @NullByte08 @Sarthak2601 @SayantanBanerjee16
NOTE for all.
The files that you are working on will also get used by other contributor on other PRs too, so you can expect merge conflicts if PR takes lot of time. Also, these are fairly difficult at the same time very good for knowledge tasks. So if you get stuck on any of these then you can leave a particular issue if you want, its completely fine.

@Sarthak2601 Sarthak2601 mentioned this issue Apr 8, 2020
Merged
8 tasks
@SayantanBanerjee16 SayantanBanerjee16 mentioned this issue Apr 8, 2020
Merged
8 tasks
rt4914 pushed a commit that referenced this issue Apr 9, 2020
@Sarthak2601
Fixes part of #632 : RecyclerView implementation using BindableAdapte…
95d601a 
…r in TopicReview/Revision (#955)

* RecyclerView implementation with BindableAdapter in TopicReview/Revision

* nit changes

* RecyclerView implementation with BindableAdapter in TopicReview/Revision

* nit changes

* nit changes
rt4914 pushed a commit that referenced this issue Apr 9, 2020
@SayantanBanerjee16
Fix part of #632 : Replace current recyclerview implementation with B…
f97edf2 
…indableAdapter usage in Help Activity (#956)

* Introduced Bindable Adapter

* Introduced onClick in the item

* Nit change
@rt4914
Copy link
Contributor Author

rt4914 commented Apr 24, 2020

@NullByte08 and @abhinavraj23 Any updates in this? If this is not doable its fine, we can unassign you from this.

@NullByte08
Copy link
Contributor

@rt4914 I am facing a lot of errors in the HomeFragment since it has two recyclerviews. Can I take some more time? I will address the issues to you that I am facing if I still don't get a solution for them.

@rt4914
Copy link
Contributor Author

rt4914 commented Apr 27, 2020

@rt4914 I am facing a lot of errors in the HomeFragment since it has two recyclerviews. Can I take some more time? I will address the issues to you that I am facing if I still don't get a solution for them.

@rt4914 Yeah sure, take your time.

@NullByte08 NullByte08 mentioned this issue Apr 29, 2020
Closed
8 tasks
@BenHenning BenHenning mentioned this issue Jun 23, 2020
Closed
@BenHenning BenHenning added this to the Beta milestone Jun 23, 2020
@anandwana001 anandwana001 mentioned this issue Nov 24, 2020
Merged
8 tasks
@jcqli
Copy link
Contributor

jcqli commented Dec 4, 2020 •
edited
Loading

@rt4914 Was TopicPlay/Lessons ever done? Happy to work on it!

@BenHenning
Copy link
Member

TopicListAdapter still needs to be removed/migrated, so that's open. Assigning you @jcqli.

@jcqli jcqli mentioned this issue Dec 15, 2020
Merged
8 tasks
@masclot
Copy link
Collaborator

masclot commented Feb 3, 2023

OngoingListAdapter was renamed PromotedStoryListAdapter in #4704
I'm going to start working on this one.

@BenHenning BenHenning mentioned this issue Feb 8, 2023
Merged
6 tasks
@seanlip seanlip added bug End user-perceivable behaviors which are not desirable. and removed issue_type_infrastructure labels Mar 28, 2023
BenHenning added a commit that referenced this issue Mar 30, 2023
@masclot @BenHenning
Fix part of #632: Move PromotedStoryListAdapter to BindableAdater (#4874
7fd290d 
)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fixes part of #632: Reimplementation of PromotedStoryListAdapter as
BindableAdater
Espresso tests are failing, but they were already failing. The errors
with the new adapter are the same. The same tests pass when using
gradle.

There was an error in MarginBindingAdapter which I'm also fixing in this
cl: MarginBindingAdapter was replacing start/end margin changes for
left/right margin changes. However, this replacement relied on getting
the layout direction from the View being changed. When the direction is
inherited and the View is not attached to a parent (recursively until
top level View), the direction defaults to LTR, no matter the
Locale/language. This situation happens for example in item Views within
RecyclerView. The change does not replace start/end any more, but sets
them through MarginLayoutParamsCompat so that it is compatible with api
< 17.

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [X] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [X] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [X] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [X] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

| Current | new |
|-|-|
|
![bindable_adapter_current](https://user-images.githubusercontent.com/103062089/217546850-30d59ac3-d996-4856-9a55-43a61e533eb2.png)
|
![bindable_adapter_new](https://user-images.githubusercontent.com/103062089/217546857-224a287d-b55a-4af9-8b05-e4855d78bd44.png)
|

Unit tests:

![recently_played_span_test](https://user-images.githubusercontent.com/103062089/217547368-f86fa7c4-c915-4257-948c-90a8e73c3ab6.png)

Espresso Tests. The run for the new version has an extra error, but it
is due to timings. In other runs it passes.

![recently_played_espresso_tests_current_vs_new](https://user-images.githubusercontent.com/103062089/217548889-21e0599e-e8b0-463d-a3f6-eeaaa93e674a.png)

@rt4914 PTAL

---------

Co-authored-by: Ben Henning <[email protected]>
@seanlip seanlip removed the user_team label Mar 30, 2023
@masclot
Copy link
Collaborator

masclot commented Mar 31, 2023

There is only one remaining class in the codebase that implements RecyclerView.Adapter: FragmentStateAdapter
This in turn is extended by two more classes: ViewPagerAdapter and MyDownloadsViewPagerAdapter

I don't know if it makes sense to refactor those. Otherwise this bug can be closed.

@adhiamboperes
Copy link
Collaborator

Unassigning masclot since his PR has been merged.
I think this needs re-triage as per @masclot's comment.

@BenHenning
Copy link
Member

Agreed--thanks @masclot & @adhiamboperes! I think we can go ahead and close this.

Uticodes pushed a commit to Uticodes/oppia-android that referenced this issue Apr 4, 2023
@masclot @BenHenning @Uticodes
Fix part of oppia#632: Move PromotedStoryListAdapter to BindableAdater (
29e786c 
oppia#4874)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fixes part of oppia#632: Reimplementation of PromotedStoryListAdapter as
BindableAdater
Espresso tests are failing, but they were already failing. The errors
with the new adapter are the same. The same tests pass when using
gradle.

There was an error in MarginBindingAdapter which I'm also fixing in this
cl: MarginBindingAdapter was replacing start/end margin changes for
left/right margin changes. However, this replacement relied on getting
the layout direction from the View being changed. When the direction is
inherited and the View is not attached to a parent (recursively until
top level View), the direction defaults to LTR, no matter the
Locale/language. This situation happens for example in item Views within
RecyclerView. The change does not replace start/end any more, but sets
them through MarginLayoutParamsCompat so that it is compatible with api
< 17.

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [X] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [X] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [X] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [X] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

| Current | new |
|-|-|
|
![bindable_adapter_current](https://user-images.githubusercontent.com/103062089/217546850-30d59ac3-d996-4856-9a55-43a61e533eb2.png)
|
![bindable_adapter_new](https://user-images.githubusercontent.com/103062089/217546857-224a287d-b55a-4af9-8b05-e4855d78bd44.png)
|

Unit tests:

![recently_played_span_test](https://user-images.githubusercontent.com/103062089/217547368-f86fa7c4-c915-4257-948c-90a8e73c3ab6.png)

Espresso Tests. The run for the new version has an extra error, but it
is due to timings. In other runs it passes.

![recently_played_espresso_tests_current_vs_new](https://user-images.githubusercontent.com/103062089/217548889-21e0599e-e8b0-463d-a3f6-eeaaa93e674a.png)

@rt4914 PTAL

---------

Co-authored-by: Ben Henning <[email protected]>
adhiamboperes added a commit that referenced this issue Apr 18, 2023
@adhiamboperes
Revert "Fix part of #632: Move PromotedStoryListAdapter to BindableAd…
b200e6d 
…ater (#4874)"

This reverts commit 7fd290d.
@adhiamboperes
Copy link
Collaborator

Reopening this since we are reverting the associated pull request.

@adhiamboperes adhiamboperes reopened this Apr 19, 2023
@adhiamboperes adhiamboperes mentioned this issue Apr 19, 2023
Merged
6 tasks
BenHenning pushed a commit that referenced this issue Apr 21, 2023
@adhiamboperes
Fix part of #2116: revert "#4874 Move PromotedStoryListAdapter to Bin…
80d2cd0 
…dableAdapter" (#4951)

This reverts commit `7fd290d68f0440d926f2d443dbd7bfb28ab20547`, fixing
part of #2116.

While testing #2116, we found that the UI for default profile view does
not display correctly, and traced the change to #4874 in [this
discusson](#2116 (comment)).

#4874 introduces a change to `MarginBindingAdapters` which utilizes
`MarginLayoutParamsCompat` to compute the margins of a layout before
drawing it. I suspect that this works for the `Promoted Story` and
`Topic Summary` because we compute the start and end margins of each
item relative to grid columns laid out on the HomeActivity, but no such
implementation exists for the Profile Chooser view. We need to do some
further investigation into this, hence the decision to revert.

We will need to test all the associated screens to make sure they still
look as expected:

![Screenshot 2023-04-19 at 18 01
28](https://user-images.githubusercontent.com/59600948/233117952-8c981fa2-2d0c-45cc-80ff-651862c1fd96.png)


## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
This was mainly a re-implimentation per #632, and the UI did not change.
Please refer to #4874 for screenshots.

The Affected screen displays okay with this revert:


![Screenshot_1681916915](https://user-images.githubusercontent.com/59600948/233125096-2242fb7e-eb9f-4b78-8db0-c22abb6b5cc3.png)

Other screens:
|   |   |
|---|---|

|![Screenshot_1681917117](https://user-images.githubusercontent.com/59600948/233126283-45a860c7-26ac-47f6-8b38-e54b1ee7cb1f.png)|![Screenshot_1681917190](https://user-images.githubusercontent.com/59600948/233126350-08033853-4434-462d-9bf3-98040ff2356d.png)|
@masclot masclot mentioned this issue May 5, 2023
Merged
6 tasks
adhiamboperes added a commit that referenced this issue Jun 24, 2023
@masclot @adhiamboperes
Fix part of #632: Simplify MarginBindingAdapters and fix some RTL cas…
1c1841b 
…es. Fix #2116: UI misalignment due to not resetting the layout. (#4965)

## Explanation

Fix #2116:
Fix part of #632:

Simplify MarginBindingAdapters.

The code is mostly the same as in PR #4874, which was reverted. This PR
fixes the bug that caused the rollback.
The rest of the reverted changes in PR #4874 will be included in a
separate PR.

The fix consists of resetting the layout params after changing the top
and bottom margins, by calling View.setLayoutParams(), as explained in
the [layout params
documentation](https://developer.android.com/reference/android/view/ViewGroup.MarginLayoutParams#bottomMargin).
The same call is not needed when calling a method instead of setting a
property.

It also fixes an RTL issue: if the view is not attached yet, there is no
layout direction information available, making the old implementation
render in LTR always.

Some screenshots:

|new|current|
|-|-|

|![home_rtl](https://user-images.githubusercontent.com/103062089/236451422-13843234-5f6c-4ed3-997a-e4b0459c3641.png)
|
![home_rtl_current](https://user-images.githubusercontent.com/103062089/236451614-55595920-ed58-4329-8d7d-7bdbc0487688.png)|

|![profile_chooser_landscape](https://user-images.githubusercontent.com/103062089/236451425-6766ed39-4a6b-4918-bf62-28bff5d05565.png)|![profile_chooser_landscape_current](https://user-images.githubusercontent.com/103062089/236451616-6979857b-f431-45cf-82c1-8db2a7618669.png)|

|![profile_chooser_portrait](https://user-images.githubusercontent.com/103062089/236451427-ba5cc671-ff03-4718-ae45-9e3cc924efeb.png)|![profile_chooser_portrait_current](https://user-images.githubusercontent.com/103062089/236451621-058ec5b9-0cdb-4713-931e-c49bd9a5edba.png)|

|![recently_played_rtl](https://user-images.githubusercontent.com/103062089/236451431-01e65a32-b448-422d-88a1-efb5326d5ab9.png)|![recently_played_rtl_current](https://user-images.githubusercontent.com/103062089/236451622-57a5cc80-f0d9-400d-8003-e9ff9735950b.png)|

Test results:
![Test
results](https://user-images.githubusercontent.com/103062089/236451433-d5a0e34f-75c0-4a6d-aeb5-88012913796f.png)


## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [ ] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

---------

Co-authored-by: Adhiambo Peres <[email protected]>
@masclot masclot mentioned this issue Jun 30, 2023
Merged
6 tasks
adhiamboperes added a commit that referenced this issue Jul 11, 2023
@masclot @adhiamboperes
Fix #632: Move PromotedStoryListAdapter to BindableAdater (#5077)
1862c4a 
Fix #632: Move PromotedStoryListAdapter to BindableAdater

This change reverts partially #4951, which in turn reverted #4874 due to
a bug.
It is a partial revert because the bug was fixed independently in the
previous PR #4965.

I added RecentlyPlayedViewModel.kt to the list of classes not needing
tests, as is usual for ViewModels. This was also part of the original
PR.

| current | new |
|- | - |

|![recently_played_ltr_current](https://github.com/oppia/oppia-android/assets/103062089/179e56c1-6548-4be0-ac03-a0b7252c3a4a)|
![recently_played_ltr_new](https://github.com/oppia/oppia-android/assets/103062089/468d13e2-f6c8-48f0-a36c-2483f6dbe728)|

|![recently_played_rtl_current](https://github.com/oppia/oppia-android/assets/103062089/cf78dd4b-4fef-4311-accb-bd03d28345d6)|
![recently_played_rtl_new](https://github.com/oppia/oppia-android/assets/103062089/90a489b4-7e2a-49b3-9cbc-1eebd6dfe491)
|

|![recently_played_ltr_landscape_current](https://github.com/oppia/oppia-android/assets/103062089/92b12fa4-54c4-4be5-8c4f-1c73e90e9cb1)|![recently_played_ltr_landscape_new](https://github.com/oppia/oppia-android/assets/103062089/39833703-9cbd-4b63-b882-5950d3154316)|

|![recently_played_rtl_landscape_current](https://github.com/oppia/oppia-android/assets/103062089/46545cf6-40aa-4f24-a977-a0a586b46445)|![recently_played_rtl_landscape_new](https://github.com/oppia/oppia-android/assets/103062089/89c6a842-e876-4dc3-8861-c2c47a938183)|

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility guide](https://github.com/op
pia/oppia-android/wiki/Accessibility-A11y-Guide))
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

---------

Co-authored-by: Adhiambo Peres <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@masclot masclot

Labels
bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Team] Core Learner and Mastery flows & UI Frontend - Android
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #632: Move PromotedStoryListAdapter to BindableAdater masclot/oppia-android