Add visual indicator of read/unread news resources #595
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rosejr
force-pushed
the
jr/track-viewed
branch
3 times, most recently
from
0a5d076
to
241e9ce
Compare
rosejr
force-pushed
the
jr/track-viewed
branch
2 times, most recently
from
9b8cc77
to
973ca2b
Compare
This moves the responsibility for joining the UserData and the NewsResources to UserNewsResourceRepository. This way, the work can be done once and shared with all consumers in a SharedFlow, rather than having each consumer perform the join itself by invoking the UseCase.
When a news resource is unread, display a dot on its card in the news feed. When the For You section has unread resources, display a dot on its icon in the navigation bar. Update the read status when a resource is opened.
rosejr
force-pushed
the
jr/track-viewed
branch
from
973ca2b
to
ebfbb5b
Compare
|
Please add screenshots of "before" and "after" to the PR description, as well as details about what this change does and why. |
dturner
requested changes
Mar 2, 2023
There was a problem hiding this comment.
Thanks for adding this. From the user's POV this works really well, especially the nav icon, which makes it super clear when you've got content you haven't read.
From an architectural POV there are some issues which are only clear now you've implemented it :)
The main pieces of feedback are:
- The cold->hot flow adds a lot of complexity for little benefit, let's remove it
- We need to decide on whether we want to keep
UserNewsRepoor refactor this into multiple use cases (jury still out on this). - Move the notification state out of
NiaAppintoNiaAppState
app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaApp.kt
Outdated
Show resolved
Hide resolved
...ogle/samples/apps/nowinandroid/core/domain/repository/CompositeUserNewsResourceRepository.kt
Outdated
Show resolved
Hide resolved
...ain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/model/UserNewsResource.kt
Outdated
Show resolved
Hide resolved
...rc/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt
Outdated
Show resolved
Hide resolved
...rc/main/java/com/google/samples/apps/nowinandroid/core/data/repository/UserDataRepository.kt
Outdated
Show resolved
Hide resolved
...ogle/samples/apps/nowinandroid/core/domain/repository/CompositeUserNewsResourceRepository.kt
Outdated
Show resolved
Hide resolved
.../foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt
Outdated
Show resolved
Hide resolved
...s/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksViewModel.kt
Outdated
Show resolved
Hide resolved
.../foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt
Outdated
Show resolved
Hide resolved
rosejr
force-pushed
the
jr/track-viewed
branch
3 times, most recently
from
8194f7a
to
519b82f
Compare
module; move UserNewsResource to model module. Implement unread dot for bookmarked articles. Keep the flows cold in UserNewsResourceRepository.
dturner
requested changes
Apr 17, 2023
...java/com/google/samples/apps/nowinandroid/core/data/repository/UserNewsResourceRepository.kt
Outdated
Show resolved
Hide resolved
...java/com/google/samples/apps/nowinandroid/core/data/repository/UserNewsResourceRepository.kt
Outdated
Show resolved
Hide resolved
...java/com/google/samples/apps/nowinandroid/core/data/repository/UserNewsResourceRepository.kt
Outdated
Show resolved
Hide resolved
|
As per internal discussion, let's also set all current news resources to read status so that the user is only notified of new items. |
dturner
approved these changes
Apr 19, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds indicator dots when a news resource is unread, and when a navigation destination has unread content. Makes necessary data layer changes to track resources that have been read. This makes it easier for users to quickly scan for content that they haven't seen already or identify content that they have already read.