-
Notifications
You must be signed in to change notification settings - Fork 520
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.
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
Fix #2546: Rename OngoingStory to PromotedStory #4704
Fix #2546: Rename OngoingStory to PromotedStory #4704
Conversation
Renamed to PromotedStoryViewModel alongside usages
Rename to promotedStoryListSummaryResultLiveData
Rename to subscribeToPromotedStoryList
Rename to PromotedStoryListAdapter
The activity previously passed into the adapter constructor had no usages.
Rename to PromotedStoryClickListener
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @adhiamboperes just had a few comments.
Regarding the layouts, when is ongoing_story_card vs. promoted_story_card used? It'd be nice to fix this last case, too, if it's the only other occurrence of "ongoing story" left.
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryClickListener.kt
Show resolved
Hide resolved
Also removed the kdoc exemptions for the same
Note: This is the layout file used to show stories in the "recently played screen", under "Played within the last week and recommended stories"
…-android into rename-ongoing-story
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @adhiamboperes just had a few comments.
Regarding the layouts, when is ongoing_story_card vs. promoted_story_card used? It'd be nice to fix this last case, too, if it's the only other occurrence of "ongoing story" left.
Note: ongoing_story_card
is used to show stories in the "recently played screen", under "Played within the last week and recommended stories", shown when a user clicks "View all promoted stories" from the home screen. I have renamed it to recently_played_story_card.xml
.
promoted_story_card.xml
is the layout file used to show promoted stories on the home page.
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryClickListener.kt
Show resolved
Hide resolved
Add documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @adhiamboperes! I think the content for the new docs is generally good, but there are a few formatting issues to address for consistency and per the style guide. PTAL.
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryClickListener.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryClickListener.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryClickListener.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryListAdapter.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryListAdapter.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryListAdapter.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have implemented the KDoc formatting suggestions, PTAL @BenHenning
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryClickListener.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryClickListener.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryClickListener.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryListAdapter.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryListAdapter.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryListAdapter.kt
Outdated
Show resolved
Hide resolved
@BenHenning I'm not sure why the workflows did not run. Could you PTAL? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @adhiamboperes! PR LGTM.
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryClickListener.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/home/recentlyplayed/PromotedStoryViewModel.kt
Outdated
Show resolved
Hide resolved
Unassigning @BenHenning since they have already approved the PR. |
Assigning @rt4914 for code owner reviews. Thanks! |
@adhiamboperes they sometimes hang, though it's a bit rare. I'll update the PR to the latest develop since it needs it, anyway, and that should retrigger a run. @rt4914 PTAL for codeowners. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adhiamboperes Suggested minor changes. Thanks.
Change the id name of ongoing_story_card_view to promoted_story_card_view in recently_played_story_card.xml. Also renamed clickOnOngoingStoryTile to clickOnPromotedStoryTile in PromotedStoryViewModel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rt4914, PTAL again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latest changes LGTM, thanks @adhiamboperes!
Unassigning @BenHenning since they have already approved the PR. |
Explanation
Fixes #2546
Rename instances of ../recentlyplayed/OngoingStory to PromotedStory.
Note:
ongoing_story_card
is used to show stories in the "recently played screen", under "Played within the last week and recommended stories", shown when a user clicks "View all promoted stories" from the home screen. I have renamed it torecently_played_story_card.xml
.promoted_story_card.xml
is the layout file used to show promoted stories on the home page.I removed
PromotedStoryClickListener
,PromotedStoryListAdapter
andPromotedStoryViewModel
from Kdoc excemption since the kdocs have been added.Essential Checklist