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

Refactored DeckPickerWidget to handle empty state visibility. #17041

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Refactored DeckPickerWidget to handle empty state visibility. #17041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 85 additions & 21 deletions AnkiDroid/src/main/java/com/ichi2/widget/deckpicker/DeckPickerWidget.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ import android.appwidget.AppWidgetManager.ACTION_APPWIDGET_UPDATE
import android.content.ComponentName
import android.content.Context
import android.content.Intent
import android.view.View
import android.widget.RemoteViews
import com.ichi2.anki.AnkiDroidApp
import com.ichi2.anki.CollectionManager.withCol
import com.ichi2.anki.CrashReportService
import com.ichi2.anki.R
import com.ichi2.anki.Reviewer
import com.ichi2.anki.analytics.UsageAnalytics
import com.ichi2.anki.isCollectionEmpty
import com.ichi2.anki.pages.DeckOptions
import com.ichi2.libanki.DeckId
import com.ichi2.widget.ACTION_UPDATE_WIDGET
Expand Down Expand Up @@ -92,52 +94,114 @@ class DeckPickerWidget : AnalyticsWidgetProvider() {
context: Context,
appWidgetManager: AppWidgetManager,
appWidgetId: AppWidgetId,
deckIds: LongArray
deckIds: LongArray?
) {
val remoteViews = RemoteViews(context.packageName, R.layout.widget_deck_picker_large)

if (deckIds == null || deckIds.isEmpty()) {
showEmptyWidget(context, appWidgetManager, appWidgetId, remoteViews)
return
}
AnkiDroidApp.applicationScope.launch {
val isCollectionEmpty = isCollectionEmpty()
if (isCollectionEmpty) {
showEmptyCollection(context, appWidgetManager, appWidgetId, remoteViews)
return@launch
}

val deckData = getDeckNamesAndStats(deckIds.toList())

remoteViews.removeAllViews(R.id.deckCollection)
if (deckData.isEmpty()) {
showEmptyWidget(context, appWidgetManager, appWidgetId, remoteViews)
return@launch
}

showDeck(context, appWidgetManager, appWidgetId, remoteViews, deckIds)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point deckIds can't be null because you already checked above to show the empty state. This allows you to avoid the null checking in showDeck by declaring the parameter as non null.

}
}

private suspend fun showDeck(
context: Context,
appWidgetManager: AppWidgetManager,
appWidgetId: AppWidgetId,
remoteViews: RemoteViews,
deckIds: LongArray?
) {
remoteViews.removeAllViews(R.id.deckCollection)

val deckData = deckIds?.let { getDeckNamesAndStats(it.toList()) }
if (deckData != null) {
for (deck in deckData) {
val deckView = RemoteViews(context.packageName, R.layout.widget_item_deck_main)

remoteViews.setViewVisibility(R.id.empty_widget, View.GONE)
remoteViews.setViewVisibility(R.id.deckCollection, View.VISIBLE)
deckView.setTextViewText(R.id.deckName, deck.name)
deckView.setTextViewText(R.id.deckNew, deck.newCount.toString())
deckView.setTextViewText(R.id.deckDue, deck.reviewCount.toString())
deckView.setTextViewText(R.id.deckLearn, deck.learnCount.toString())

val isEmptyDeck = deck.newCount == 0 && deck.reviewCount == 0 && deck.learnCount == 0

if (!isEmptyDeck) {
val intent = Intent(context, Reviewer::class.java).apply {
val intent = if (!isEmptyDeck) {
Intent(context, Reviewer::class.java).apply {
action = Intent.ACTION_VIEW
putExtra("deckId", deck.deckId)
addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)
}
xenonnn4w marked this conversation as resolved.
Show resolved Hide resolved
val pendingIntent = PendingIntent.getActivity(
context,
deck.deckId.toInt(),
intent,
PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE
)
deckView.setOnClickPendingIntent(R.id.deckName, pendingIntent)
} else {
val intent = DeckOptions.getIntent(context, deck.deckId)
val pendingIntent = PendingIntent.getActivity(
context,
deck.deckId.toInt(),
intent,
PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE
)
deckView.setOnClickPendingIntent(R.id.deckName, pendingIntent)
DeckOptions.getIntent(context, deck.deckId)
}

val pendingIntent = PendingIntent.getActivity(
context,
deck.deckId.toInt(),
intent,
PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE
)

deckView.setOnClickPendingIntent(R.id.deckName, pendingIntent)
remoteViews.addView(R.id.deckCollection, deckView)
}
appWidgetManager.updateAppWidget(appWidgetId, remoteViews)
}

appWidgetManager.updateAppWidget(appWidgetId, remoteViews)
}

private fun showEmptyCollection(
context: Context,
appWidgetManager: AppWidgetManager,
appWidgetId: AppWidgetId,
remoteViews: RemoteViews
) {
remoteViews.setTextViewText(R.id.empty_widget, context.getString(R.string.app_not_initialized_new))
Copy link
Member

@lukstbit lukstbit Sep 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested the no collection scenario and the user experience is not great here.

If I have a collection with one deck that I register in the widget and then go to the app and delete the deck, when I return to the widget I see the message: AnkiDroid is not initialized yet. Please open AnkiDroid and try again. The issues:

  • text is misleading
  • if I'm going to the app and come back I still see the message
  • if I'm going to the app and add decks and come back I still see the message
  • touching the widget does nothing

I have to long click the widget to bring the system configure button to do anything.

So I would recommend to create a special string for this case(example: Collection is empty, use app to add decks then reconfigure), when the collection is empty and also make that text clickable to show the configure screen.

remoteViews.setViewVisibility(R.id.empty_widget, View.VISIBLE)
remoteViews.setViewVisibility(R.id.deckCollection, View.GONE)

appWidgetManager.updateAppWidget(appWidgetId, remoteViews)
}

private fun showEmptyWidget(
context: Context,
appWidgetManager: AppWidgetManager,
appWidgetId: AppWidgetId,
remoteViews: RemoteViews
) {
remoteViews.setViewVisibility(R.id.empty_widget, View.VISIBLE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are using the empty TextView to show a message for when the collection is empty(by setting a specific text there) than we should also set the missing deck text here.

remoteViews.setViewVisibility(R.id.deckCollection, View.GONE)

val configIntent = Intent(context, DeckPickerWidgetConfig::class.java).apply {
putExtra(AppWidgetManager.EXTRA_APPWIDGET_ID, appWidgetId)
flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TASK
}
val configPendingIntent = PendingIntent.getActivity(
context,
appWidgetId,
configIntent,
PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE
)
remoteViews.setOnClickPendingIntent(R.id.empty_widget, configPendingIntent)

appWidgetManager.updateAppWidget(appWidgetId, remoteViews)
}

/**
Expand Down
11 changes: 11 additions & 0 deletions AnkiDroid/src/main/res/layout/widget_deck_picker_large.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@
android:focusable="false"
android:theme="@style/Theme.Material3.DynamicColors.DayNight">

<TextView
android:id="@+id/empty_widget"
style="@style/TextAppearance.AppCompat.Medium"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="7dp"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As material design uses multiples of 4 for dimensions we should also use this for marginstart(8dp).

android:gravity="center"
android:padding="20dp"
android:text="@string/empty_widget"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current text is "Missing deck. Please reconfigure". I think it may better to change the last part to add either Touch or Click to tell the user how he can act on the issue:

"Missing deck. Touch to reconfigure"
"Missing deck. Click to reconfigure"

android:visibility="gone" />

<LinearLayout
android:id="@+id/deckCollection"
android:layout_width="match_parent"
Expand Down