Fix #1152: LessonThumbnailImageView #1554

rt4914 · 2020-08-05T02:58:06Z

Explanation

This PR create a new class LessonThumbnailImageView such that we can use this custom image view in xml files, pass the required information and load the svg/png in imageview.

Please go through the entire description and review this PR in this format to make things easier.

Screenshots

How to test

Current develop content

Run the app in Mobile & Tablet.
Open following screens in landscape + portrait mode: Home Screen, TopicInfo, StoryActivity, TopicRevision, RecentlyPlayed, ProfileProgress (from navigation header).
Make sure you see all these above mentioned screen on mobile (portrait + landscape) and tablet (portrait + landscape).

Alpha content

Uninstall the current app.
In domain/assets folder, remove all json files and paste all json files which I have shared with you personally.
Run the app in Mobile & Tablet.
Open following screens in landscape + portrait mode: Home Screen, TopicInfo, StoryActivity, TopicRevision, RecentlyPlayed, ProfileProgress (from navigation header).
Replace StoryProgressTestHelper code with https://gist.github.com/rt4914/6f31bae848eca861fa4080f0590d1b85
Inject StoryProgressTestHelper in HomeFragmentPresenter, use markOngoingTopicList to mark topic partial progress, open navigation drawer -> profile progress -> click on Ongoing Topics item.
Inject StoryProgressTestHelper in HomeFragmentPresenter, use markCompletedStoryList to mark story progress, open navigation drawer -> profile progress -> click on Completed Stories item.
Make sure you see all these above mentioned screen on mobile (portrait + landscape) and tablet (portrait + landscape).

How to review code

Xml Files:

All xml files change ImageView to LessonThumbnailImageView and add app:entityId, app:entityType, app:lessonThumbnail and removed android:src, android:background.
Or there can be reformatting changes in some xml files.

All other files:

Introduced StoryHtmlParserEntityType or TopicHtmlParserEntityType in files so that we can pass entityType to viewmodels so that we can use it in xml file.
Also, introduced utility layer changes to support svg rendering.

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
The PR follows the style guide.
The PR does not contain any unnecessary auto-generated code from Android Studio.
The PR is made from a branch that's not called "develop".
The PR is made from a branch that is up-to-date with "develop".
The PR's branch is based on "develop" and not on any other branch.
The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

…t-part-1

BenHenning · 2020-08-05T03:06:42Z

@rt4914 do you have the version of LessonThumbnailImageView that more closely follows ImageRegionSelectionInteractionView and has the error above to compare?

rt4914 · 2020-08-05T03:09:27Z

@rt4914 do you have the version of LessonThumbnailImageView that more closely follows ImageRegionSelectionInteractionView and has the error above to compare?

Created this gist for reference: https://gist.github.com/rt4914/ea26d973a9a62b682bc93e61c06e71ef

BenHenning · 2020-08-05T03:50:01Z

Thanks. It seems like it turns out Glide supports custom models into custom targets, or known models (e.g. bitmap & drawables) into custom targets & views. It doesn't support custom models into views for some reason, so while I don't like it, it seems like we have to do the SVG -> PictureDrawable conversion instead of just Picture despite that seeming a bit cleaner.

While I don't like the new ImageLoader changes, it works & it'll become cleaner once #1039 is fixed. See my commit to this branch for a fix.

rt4914 · 2020-08-05T04:19:38Z

Thanks. It seems like it turns out Glide supports custom models into custom targets, or known models (e.g. bitmap & drawables) into custom targets & views. It doesn't support custom models into views for some reason, so while I don't like it, it seems like we have to do the SVG -> PictureDrawable conversion instead of just Picture despite that seeming a bit cleaner.

While I don't like the new ImageLoader changes, it works & it'll become cleaner once #1039 is fixed. See my commit to this branch for a fix.

Okay. Thanks for the commit.

anandwana001

Story Activity Items and lessons on topic screen not clickable after rotation

Open Story Activity or any screen where we have images and rotate, try scrolling up and down, or just try scrolling a bit fast

kotlin.UninitializedPropertyAccessException: lateinit property thumbnailDownloadUrlTemplate has not been initialized
        at org.oppia.app.customview.LessonThumbnailImageView.loadImage(LessonThumbnailImageView.kt:78)
        at org.oppia.app.customview.LessonThumbnailImageView.loadLessonThumbnail(LessonThumbnailImageView.kt:68)
        at org.oppia.app.customview.LessonThumbnailImageView.checkIfLoadingIsPossible(LessonThumbnailImageView.kt:62)
        at org.oppia.app.customview.LessonThumbnailImageView.setLessonThumbnail(LessonThumbnailImageView.kt:57)
        at org.oppia.app.databinding.StoryChapterViewBindingImpl.executeBindings(StoryChapterViewBindingImpl.java:233)

Entering first time into the application, recently played is coming empty.

// works correctly
storyProgressTestHelper.markCompletedStoryList(profileId,false)

// empty recently played list but view all option available
storyProgressTestHelper.markCompletedStoryList(profileId,true)

// works correctly
storyProgressTestHelper.markOngoingTopicList(profileId,false)

// empty recently played list but view all option available
storyProgressTestHelper.markOngoingTopicList(profileId,true)

Story Activity in landscape, not showing images

Difference - Recently played difference from home screen and from profile screen

Scrolling recently played list on home screen after coming from the exploration, in order to get a lesson to get added in the recently played list

2020-08-06 11:33:02.706 17369-17369/org.oppia.app E/AndroidRuntime: FATAL EXCEPTION: main
    Process: org.oppia.app, PID: 17369
    java.lang.RuntimeException: Failed to call observer method
        at androidx.lifecycle.ClassesInfoCache$MethodReference.invokeCallback(ClassesInfoCache.java:226)
        at androidx.lifecycle.ClassesInfoCache$CallbackInfo.invokeMethodsForEvent(ClassesInfoCache.java:194)
        at androidx.lifecycle.ClassesInfoCache$CallbackInfo.invokeCallbacks(ClassesInfoCache.java:185)
        at androidx.lifecycle.ReflectiveGenericLifecycleObserver.onStateChanged(ReflectiveGenericLifecycleObserver.java:37)
        at androidx.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent(LifecycleRegistry.java:361)
        at androidx.lifecycle.LifecycleRegistry.forwardPass(LifecycleRegistry.java:300)
        at androidx.lifecycle.LifecycleRegistry.sync(LifecycleRegistry.java:339)
        at androidx.lifecycle.LifecycleRegistry.moveToState(LifecycleRegistry.java:145)
        at androidx.lifecycle.LifecycleRegistry.handleLifecycleEvent(LifecycleRegistry.java:131)
        at androidx.fragment.app.Fragment.performStart(Fragment.java:2732)
        at androidx.fragment.app.FragmentStateManager.start(FragmentStateManager.java:331)
        at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1239)
        at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1398)
        at androidx.fragment.app.FragmentManager.moveFragmentToExpectedState(FragmentManager.java:1476)
        at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1541)
        at androidx.fragment.app.FragmentManager.dispatchStateChange(FragmentManager.java:2858)
        at androidx.fragment.app.FragmentManager.dispatchStart(FragmentManager.java:2818)
        at androidx.fragment.app.Fragment.performStart(Fragment.java:2736)
        at androidx.fragment.app.FragmentStateManager.start(FragmentStateManager.java:331)
        at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1239)
        at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1398)
        at androidx.fragment.app.FragmentManager.moveFragmentToExpectedState(FragmentManager.java:1476)
        at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1541)
        at androidx.fragment.app.FragmentManager.dispatchStateChange(FragmentManager.java:2858)
        at androidx.fragment.app.FragmentManager.dispatchStart(FragmentManager.java:2818)
        at androidx.fragment.app.FragmentController.dispatchStart(FragmentController.java:258)
        at androidx.fragment.app.FragmentActivity.onStart(FragmentActivity.java:550)
        at androidx.appcompat.app.AppCompatActivity.onStart(AppCompatActivity.java:201)
        at android.app.Instrumentation.callActivityOnStart(Instrumentation.java:1391)
        at android.app.Activity.performStart(Activity.java:7157)
        at android.app.ActivityThread.handleStartActivity(ActivityThread.java:2937)
        at android.app.servertransaction.TransactionExecutor.performLifecycleSequence(TransactionExecutor.java:180)
        at android.app.servertransaction.TransactionExecutor.cycleToPath(TransactionExecutor.java:165)
        at android.app.servertransaction.TransactionExecutor.executeLifecycleState(TransactionExecutor.java:142)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:70)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1808)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6669)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)
     Caused by: kotlin.UninitializedPropertyAccessException: lateinit property thumbnailDownloadUrlTemplate has not been initialized
        at org.oppia.app.customview.LessonThumbnailImageView.loadImage(LessonThumbnailImageView.kt:78)
        at org.oppia.app.customview.LessonThumbnailImageView.loadLessonThumbnail(LessonThumbnailImageView.kt:68)
2020-08-06 11:33:02.706 17369-17369/org.oppia.app E/AndroidRuntime:     at org.oppia.app.customview.LessonThumbnailImageView.checkIfLoadingIsPossible(LessonThumbnailImageView.kt:62)
        at org.oppia.app.customview.LessonThumbnailImageView.setEntityType(LessonThumbnailImageView.kt:52)
        at org.oppia.app.databinding.TopicInfoFragmentBindingImpl.executeBindings(TopicInfoFragmentBindingImpl.java:416)
        at androidx.databinding.ViewDataBinding.executeBindingsInternal(ViewDataBinding.java:473)
        at androidx.databinding.ViewDataBinding.executePendingBindings(ViewDataBinding.java:445)
        at androidx.databinding.ViewDataBinding$OnStartListener.onStart(ViewDataBinding.java:1687)
        at java.lang.reflect.Method.invoke(Native Method)
        at androidx.lifecycle.ClassesInfoCache$MethodReference.invokeCallback(ClassesInfoCache.java:216)
        	... 41 more

app/src/main/res/layout-land/ongoing_topic_item.xml

app/src/main/res/layout-sw600dp-port/ongoing_topic_item.xml

app/src/main/res/layout-sw600dp-land/topic_revision_summary_view.xml

app/src/main/res/layout-sw600dp-port/topic_revision_summary_view.xml

rt4914 · 2020-08-06T08:41:43Z

@anandwana001 The error that you mentioned is something that I was discussing with @BenHenning yesterday but somehow I was not able encounter it again. So, its great that you mentioned it here, we will discuss this in today's meeting.

Related to no items visible in HomeScreen for promoted stories, I have filed a separate issue for that #1562

Related to difference in items in home screen for promoted stories and recently played screen, that implementation is correct because in HomeScreen-PromotedStories we show only those stories which were played in last 7 days but in RecentlyPlayedActivity we show two different sections, one last 7 days and other in last month. You can notice that in your screenshots too.

Regarding no-thumbnail images in StoryActivity - Landscape, that's correct implementation, we don't show thumbnail images in landscape mode for StoryActivity. Reference: https://xd.adobe.com/view/ee9e607b-dbd6-4372-48dc-b687d32af3da-98af/screen/cce938d6-e079-4761-bac0-8bde22ac6348/

Thanks.

anandwana001

LGTM, Thanks

rt4914 · 2020-08-07T03:22:51Z

cc #1571

Rajat Talesra added 3 commits July 31, 2020 05:09

LessonThumbnail Image View

c7421c4

Draft approach for Lesson thumbnail

a219e1a

Merge remote-tracking branch 'upstream/develop' into thumbnail-suppor…

b66c0c8

…t-part-1

rt4914 requested a review from BenHenning August 5, 2020 02:59

rt4914 assigned BenHenning Aug 5, 2020

BenHenning assigned rt4914 Aug 5, 2020

Possible fix for Glide not liking custom models bound into views.

32adc09

Working implementation

c45a1d3

BenHenning removed their assignment Aug 6, 2020

Rajat Talesra added 7 commits August 6, 2020 08:48

Added thumbnail support to Recently played screen, subtopics

c6172f8

StoryTestHelper

06b6021

Merge remote-tracking branch 'upstream/develop' into HEAD

32ee394

Profile Progress items

643fe62

Updated json files

08babba

Reset progress test file

c165b1f

Nit changes

26967f2

rt4914 changed the title ~~Fix part of #1152: LessonThumbnailImageView [DO NOT MERGE]~~ Fix #1152: LessonThumbnailImageView Aug 6, 2020

rt4914 requested a review from anandwana001 August 6, 2020 04:49

rt4914 assigned anandwana001 and unassigned rt4914 Aug 6, 2020

anandwana001 suggested changes Aug 6, 2020

View reviewed changes

anandwana001 assigned rt4914 and unassigned anandwana001 Aug 6, 2020

Nit changes

cb5403b

rt4914 removed their assignment Aug 6, 2020

rt4914 assigned anandwana001 Aug 6, 2020

rt4914 mentioned this pull request Aug 6, 2020

Fix #1562: Bug fix promoted story list #1564

Merged

8 tasks

anandwana001 approved these changes Aug 6, 2020

View reviewed changes

anandwana001 assigned rt4914 and unassigned anandwana001 Aug 6, 2020

Rajat Talesra added 3 commits August 7, 2020 07:44

Resolve merge conflicts

c8f9072

Added thumbnail to new files

03a71e7

Added todo

01ff0aa

rt4914 merged commit c735bb3 into develop Aug 7, 2020

rt4914 deleted the thumbnail-support-part-1 branch August 7, 2020 03:36

rt4914 mentioned this pull request Aug 7, 2020

Introduce Changes to proto and test cases in domain layer #1476

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #1152: LessonThumbnailImageView #1554

Fix #1152: LessonThumbnailImageView #1554

rt4914 commented Aug 5, 2020 •

edited by anandwana001

BenHenning commented Aug 5, 2020

rt4914 commented Aug 5, 2020

BenHenning commented Aug 5, 2020

rt4914 commented Aug 5, 2020

anandwana001 left a comment

rt4914 commented Aug 6, 2020

anandwana001 left a comment

rt4914 commented Aug 7, 2020

Fix #1152: LessonThumbnailImageView #1554

Fix #1152: LessonThumbnailImageView #1554

Conversation

rt4914 commented Aug 5, 2020 • edited by anandwana001

Explanation

Screenshots

How to test

Current develop content

Alpha content

How to review code

Xml Files:

All other files:

Checklist

BenHenning commented Aug 5, 2020

rt4914 commented Aug 5, 2020

BenHenning commented Aug 5, 2020

rt4914 commented Aug 5, 2020

anandwana001 left a comment

Choose a reason for hiding this comment

rt4914 commented Aug 6, 2020

anandwana001 left a comment

Choose a reason for hiding this comment

rt4914 commented Aug 7, 2020

rt4914 commented Aug 5, 2020 •

edited by anandwana001