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.

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

Add app notifications to home screen #1760

Merged
merged 2 commits into from
Jul 6, 2022

Conversation

nielsvanvelzen
Copy link
Member

This one was on my list for a while now. Right now it implements a single message but some others I'm considering are:

Changes

  • Show "notifications" on the home screen
    • Managed by NotificationRepository
  • Add notification about the UI mode of the system (detect TV devices, show incompatibility warning)
  • Show notification in a home section
    • Autohides when empty
    • Click on a notification to hide it (until you restart the app)

Screenshots
image

Issues

@nielsvanvelzen nielsvanvelzen changed the title Home notifications Add app notifications to home screen Jun 7, 2022
xmlns:tools="http://schemas.android.com/tools"
android:layout_width="280dp"
android:layout_height="90dp"
android:background="@color/cardview_dark_background"

Check warning

Code scanning / Android Lint

Overdraw: Painting regions more than once

Possible overdraw: Root element paints background @color/cardview_dark_background with a theme that also paints a background (inferred theme is @style/Theme.Jellyfin)
override fun getOldListSize(): Int = data.size
override fun getNewListSize(): Int = items.size

override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean = areContentsTheSame(oldItemPosition, newItemPosition)

Check notice

Code scanning

Line detected, which is longer than the defined maximum line length in the code style.

Line detected, which is longer than the defined maximum line length in the code style.
override fun getNewListSize(): Int = items.size

override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean = areContentsTheSame(oldItemPosition, newItemPosition)
override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean = data[oldItemPosition] == items[newItemPosition]

Check notice

Code scanning

Line detected, which is longer than the defined maximum line length in the code style.

Line detected, which is longer than the defined maximum line length in the code style.
update(notificationsRepository.notifications.value.isEmpty())
}

override fun onItemClicked(itemViewHolder: Presenter.ViewHolder?, item: Any?, rowViewHolder: RowPresenter.ViewHolder?, row: Row?) {

Check notice

Code scanning

Line detected, which is longer than the defined maximum line length in the code style.

Line detected, which is longer than the defined maximum line length in the code style.
@thornbill
Copy link
Member

I think this is a great idea! Should we maybe make it the full width of the screen?

@nielsvanvelzen
Copy link
Member Author

There can be multiple notifications, which would then show next to each other. Making it full width would look weird on a TV device. Open for suggestions though.

@thornbill
Copy link
Member

Maybe use a bulleted list in a single full-width box for multiple? I don't know really... just trying to think of some conventions I have seen used before. That wouldn't really work if they are individually dismissable though.

I'm sure it would be fine as is too. 😅

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Jun 9, 2022
@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Jun 14, 2022
@nielsvanvelzen nielsvanvelzen merged commit e770872 into jellyfin:master Jul 6, 2022
@nielsvanvelzen nielsvanvelzen deleted the home-notifications branch July 6, 2022 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants