-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Refactored DeckPickerWidget
to handle empty state visibility.
#17041
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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) | ||
} | ||
} | ||
|
||
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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" |
||
android:visibility="gone" /> | ||
|
||
<LinearLayout | ||
android:id="@+id/deckCollection" | ||
android:layout_width="match_parent" | ||
|
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.
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.