[Compose] feat: Redesign shared deck download by criticalAY · Pull Request #21150 · ankidroid/Anki-Android

criticalAY · 2026-05-26T15:04:56Z

Purpose / Description

Superseds Redesign shared deck download UI with Material 3 #20962
Related to Redesign shared deck download UI with Material 3 #20962, there are few nitpicks that I have done in the design and this is in compose

Fixes

NA

Approach

First I extracted logic to viewmodel then I used composables to create fresh views proposed by @ColbyCabrera

How Has This Been Tested?

Pixel 10 and Emulator and tests

Screen_recording_20260526_203244.mp4

Learning (optional, can help others)

NA

Checklist

Please, go through these checks before submitting the PR.

You have a descriptive commit message with a short title (first line, max 50 chars).
You have commented your code, particularly in hard-to-understand areas
You have performed a self-review of your own code
UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
UI Changes: You have tested your change using the Google Accessibility Scanner

Adds the Kotlin Compose Compiler plugin to :AnkiDroid, pulls in Compose BOM 2026.04.01 with material3, ui-tooling, activity-compose and the lifecycle-compose pair, and opts in to ExperimentalMaterial3Api at the module level so screens don't need @OptIn annotations. The Compose Compiler attaches to every Kotlin compilation in the module, so testFixtures (which has no @composable code) needs the runtime on its compileOnly classpath to satisfy the plugin's version check. Also relaxes ktlint's function-naming rule for @composable functions so PascalCase composables don't trip the linter.

…package Creates com.ichi2.anki.shareddeck and moves SharedDecksActivity, SharedDecksDownloadFragment and the DownloadFile data class into it, so all shared-decks code lives in one place. External references in the manifest, IntentHandler, DeckPicker, ActivityList and the two tests are updated to the new import path. SharedDecksDownloadViewModel exposes a StateFlow<SharedDecksDownloadUiState> covering every view-visible field on the XML screen. The fragment subscribes once in onViewCreated and renders the whole state in renderState(); progress updates, broadcast results and user actions are sent as SharedDecksDownloadEvent instances through a single onEvent(event) entry point rather than a per-mutation method. System work (DownloadManager, BroadcastReceiver, polling, file ops) stays in the fragment. No behaviour change.

github-actions · 2026-05-26T15:05:12Z

Important

Maintainers: This PR contains Strings changes

Sync Translations before merging this PR and wait for the action to complete
Review and merge the auto-generated PR in order to sync all user-submitted translations
Sync Translations again and merge the PR so the huge automated string changes caused by merging this PR are by themselves and easy to review

Co-Authored-By: Colby Cabrera <colbycabrera.wd@gmail.com>

david-allison

If you're going to fixup someone's PR, you need to be very careful to avoid offence.

Ideally reach out to the contributor first
- (Not sure if you did; this one completely slipped by my review queue and a ton of effort went into it, and I should have done better)
Use the .patch url to ensure you get the author correct
- As it stands, the co-author is incorrect
- sample: https://github.com/ankidroid/Anki-Android/commit/1de48393876fbf764939692e10b07adb691c04ae.patch
Ideally provide author, and take the co-author, I'm unsure on the impact, but it's a nice show of goodwill

criticalAY · 2026-05-26T23:38:34Z

I did notice the co-auth and sure I have no issue keep OP as the auther and I did take permission before proceeding @ColbyCabrera was happy I don't mind taking all this to his PR and closing this one later I am okay here whatever @ColbyCabrera thinks is good since its his work in terms of design and idea

david-allison · 2026-05-26T23:39:37Z

All good, cheers!

ColbyCabrera · 2026-05-26T23:51:47Z

If you're going to fixup someone's PR, you need to be very careful to avoid offence.

Ideally reach out to the contributor first

(Not sure if you did; this one completely slipped by my review queue and a ton of effort went into it, and I should have done better)

Use the .patch url to ensure you get the author correct

As it stands, the co-author is incorrect

sample: https://github.com/ankidroid/Anki-Android/commit/1de48393876fbf764939692e10b07adb691c04ae.patch

Ideally provide author, and take the co-author, I'm unsure on the impact, but it's a nice show of goodwill

No worries at all on it slipping by! (probably a good thing with the compose first announcement timing lol). I have no issues keeping it here and being co-author, he asked beforehand 🫡

david-allison

Partial review

david-allison · 2026-05-26T23:39:51Z

  <code_scheme name="Project" version="173">
    <option name="CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND" value="999" />
    <option name="NAMES_COUNT_TO_USE_IMPORT_ON_DEMAND" value="999" />
-    <option name="PACKAGES_TO_USE_IMPORT_ON_DEMAND">


I didn't actually make this change idk how it happened will revert

david-allison · 2026-05-26T23:39:59Z

 package com.ichi2.anki

 import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.ichi2.anki.shareddeck.DownloadFile


david-allison · 2026-05-26T23:49:21Z

+    val downloadedFmt =
+        remember(downloadedBytes, configuration) {
+            Formatter.formatFileSize(context, downloadedBytes)
+        }


Why is this being remembered if it effectively changes per-tick?

david-allison · 2026-05-26T23:53:10Z

+import com.ichi2.anki.R
+
+@Composable
+internal fun rememberDownloadSizeRange(


Should this be remember? It doesn't return a mutable object:

Jetpack Compose framework development and Library development MUST prefix any @Composable factory function that internally remember {}s and returns a mutable object with the prefix remember.

App development SHOULD follow this same convention.

https://android.googlesource.com/platform/frameworks/support/+/androidx-main/compose/docs/compose-api-guidelines.md#naming-composable-functions-that-remember-the-objects-they-return

david-allison · 2026-05-26T23:55:14Z

        supportActionBar?.setDisplayShowHomeEnabled(true)

-        downloadManager = getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager
+        downloadManager = getSystemService(DOWNLOAD_SERVICE) as DownloadManager


getSystemService<DownloadManager>

david-allison · 2026-05-26T23:57:19Z

+                val typedValue = TypedValue()
+                theme.resolveAttribute(R.attr.appBarColor, typedValue, true)
+                binding.webviewToolbar.setBackgroundColor(typedValue.data)


MaterialColors.getColor should work here

david-allison · 2026-05-26T23:57:52Z

        binding.webView.webViewClient = webViewClient
        onBackPressedDispatcher.addCallback(onBackPressedCallback)
+
+        supportFragmentManager.addOnBackStackChangedListener {


nit: brief comment + would recommend moving into a setupX method (as ReviewerFragment does

david-allison · 2026-05-26T23:59:42Z

+    private fun onScreenAction(action: SharedDecksDownloadAction) {
+        when (action) {
+            SharedDecksDownloadAction.CancelConfirmed -> {
+                Timber.i("SharedDecksDownloadFragment:: cancelling download deck")
+                confirmCancelDownload()
+            }
+            SharedDecksDownloadAction.ImportClicked -> {
+                Timber.i("SharedDecksDownloadFragment:: opening downloaded deck")
+                openDownloadedDeck(requireContext())
+            }
+            SharedDecksDownloadAction.TryAgainClicked -> {
+                Timber.i("SharedDecksDownloadFragment:: retrying download")
+                retryDownload()
+            }
+            SharedDecksDownloadAction.OpenInBrowserClicked -> {
+                Timber.i("SharedDecksDownloadFragment:: open in browser clicked")
+                openInBrowser()
+            }
+        }
+    }


🐉 Let's define a naming convention. Here was my attempt:

refactor(deck-picker): introduce UiEvent #20683

david-allison · 2026-05-27T00:01:14Z

        super.onViewCreated(view, savedInstanceState)

-        val fileToBeDownloaded = arguments?.getSerializableCompat<DownloadFile>(DOWNLOAD_FILE)!!
+        fileToBeDownloaded = arguments?.getSerializableCompat<DownloadFile>(DOWNLOAD_FILE)!!


nit: my preference is:

I think requireSerializableCompat would need to be defined

private val fileToBeDownloaded get() = requireArguments().requireSerializableCompat<DownloadFile>(DOWNLOAD_FILE)

david-allison · 2026-05-27T00:02:22Z

                checkDownloadProgress()
-
-                // Keep checking download progress at intervals of 1 second.
                handler.postDelayed(this, DOWNLOAD_PROGRESS_CHECK_DELAY)


comment felt OK, your call

criticalAY added 4 commits May 26, 2026 03:44

feat(compose): add Theme bridge and preview infrastructure

26e169f

feat(compose): add MaterialTheme.dimensions spacing tokens

40e43b9

criticalAY added the Blocked by dependency Currently blocked by some other dependent / related change label May 26, 2026

github-actions Bot added the Needs Review label May 26, 2026

github-actions Bot added the Strings label May 26, 2026

criticalAY force-pushed the sharedeck-compose branch from cbafb4f to 8a12ea2 Compare May 26, 2026 15:22

feat(shared-decks): redesign download screen in Compose

b5be3b0

Co-Authored-By: Colby Cabrera <colbycabrera.wd@gmail.com>

criticalAY force-pushed the sharedeck-compose branch from 8a12ea2 to b5be3b0 Compare May 26, 2026 16:29

david-allison requested changes May 26, 2026

View reviewed changes

david-allison reviewed May 27, 2026

View reviewed changes

david-allison mentioned this pull request May 29, 2026

Incremental Jetpack Compose adoption #21138

Open

19 tasks

Uh oh!

Conversation

criticalAY commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Learning (optional, can help others)

Checklist

Uh oh!

github-actions Bot commented May 26, 2026

Uh oh!

david-allison left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criticalAY commented May 26, 2026

Uh oh!

david-allison commented May 26, 2026

Uh oh!

ColbyCabrera commented May 26, 2026

Uh oh!

david-allison left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-allison May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

criticalAY commented May 26, 2026 •

edited

Loading

david-allison left a comment •

edited

Loading

david-allison May 27, 2026 •

edited

Loading