Skip to content

Conversation

@Haz3-jolt
Copy link
Member

@Haz3-jolt Haz3-jolt commented Jun 22, 2025

Purpose / Description

Introduces the changeNoteTypeDialog and changeNoteTypeViewModel

Fixes

How Has This Been Tested?

Pixel 9 Pro (Android 1

Screenshot 2025-07-19 at 08 41 01 Screenshot 2025-07-19 at 08 41 45 Screenshot 2025-07-19 at 08 42 37

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
    • ❗️ It doesn't like the fixed height of our spinner control - we should add an issue

@github-actions
Copy link
Contributor

Important

Maintainers: This PR contains Strings changes

  1. Sync Translations before merging this PR and wait for the action to complete
  2. Review and merge the auto-generated PR in order to sync all user-submitted translations
  3. 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

@Haz3-jolt Haz3-jolt force-pushed the changeNT branch 3 times, most recently from 94aef84 to 9fd2a79 Compare June 22, 2025 20:15
@Haz3-jolt Haz3-jolt marked this pull request as ready for review June 22, 2025 20:37
Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

People have been wanting this for AGES, thanks so much!! 🥳🥳🥳🥳🥳

Looks great, cheers!

SO many users will love this, let me know if you want me to dig in and rebase/force push to get this up to scratch

Crash (resolved)

1 card: Basic

  • Go to 'Templates'
  • Change' Type' to 'Basic & Reversed'

Crash

2025-06-22 21:38:42.737 24328-24328 ChangeNote...ateSpinner com.ichi2.anki.debug                 D  Updating card mapping: old template 0 -> new template 0
2025-06-22 21:38:42.738 24328-24328 ChangeNote...ateSpinner com.ichi2.anki.debug                 D  Updating card mapping: old template 1 -> new template null
2025-06-22 21:38:42.738 24328-24328 ChangeNoteTypeViewModel com.ichi2.anki.debug                 W  Attempted to update card mapping before initialization
2025-06-22 21:38:42.757 24328-24328 ThrowableFilterService  com.ichi2.anki.debug                 V  exceptionIsUnwanted - examining IndexOutOfBoundsException
2025-06-22 21:38:42.757 24328-24328 ThrowableFilterService  com.ichi2.anki.debug                 V  exceptionIsUnwanted - exception was wanted
2025-06-22 21:38:42.757 24328-24328 UsageAnalytics          com.ichi2.anki.debug                 D  sendAnalyticsException() description/fatal: java.lang.IndexOutOfBoundsException: Index 1 out of bounds for length 1/true
2025-06-22 21:38:42.763 24328-24328 ACRA                    com.ichi2.anki.debug                 E  ACRA caught a IndexOutOfBoundsException for com.ichi2.anki.debug
                                                                                                    java.lang.IndexOutOfBoundsException: Index 1 out of bounds for length 1
                                                                                                    	at jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64)
                                                                                                    	at jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70)
                                                                                                    	at jdk.internal.util.Preconditions.checkIndex(Preconditions.java:266)
                                                                                                    	at java.util.Objects.checkIndex(Objects.java:385)
                                                                                                    	at java.util.ArrayList.get(ArrayList.java:434)
                                                                                                    	at com.ichi2.anki.dialogs.ChangeNoteTypeViewModel.getDiscardedCards(ChangeNoteTypeViewModel.kt:261)
                                                                                                    	at com.ichi2.anki.dialogs.ChangeNoteTypeViewModel$getDiscardedCards$1.invokeSuspend(Unknown Source:20)
                                                                                                    	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
                                                                                                    	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
                                                                                                    	at android.os.Handler.handleCallback(Handler.java:995)
                                                                                                    	at android.os.Handler.dispatchMessage(Handler.java:103)
                                                                                                    	at android.os.Looper.loopOnce(Looper.java:248)
                                                                                                    	at android.os.Looper.loop(Looper.java:338)
                                                                                                    	at android.app.ActivityThread.main(ActivityThread.java:9067)
                                                                                                    	at java.lang.reflect.Method.invoke(Native Method)
                                                                                                    	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:593)
                                                                                                    	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:932)
                                                                                                    	Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@6e28089, Dispatchers.Main.immediate]

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label Jun 23, 2025
@Haz3-jolt Haz3-jolt added the Needs a new dev For PR that were good start but the original dev' left. Any dev can take over and finish it. label Jun 24, 2025
@david-allison david-allison marked this pull request as draft June 24, 2025 02:36
@david-allison david-allison self-assigned this Jun 24, 2025
@david-allison

This comment was marked as resolved.

@david-allison david-allison force-pushed the changeNT branch 5 times, most recently from 8e5adbe to 0e5bebe Compare June 27, 2025 22:31
@Haz3-jolt Haz3-jolt added this to the 2.22 release milestone Jun 28, 2025
@david-allison david-allison removed Needs Author Reply Waiting for a reply from the original author Needs a new dev For PR that were good start but the original dev' left. Any dev can take over and finish it. Has Conflicts labels Jun 29, 2025
@david-allison david-allison removed their assignment Jun 29, 2025
@david-allison
Copy link
Member

The colors look off in night mode

Screenshot 2025-12-04 at 13 30 29

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

The main blocker is the color in night mode.

I think the rest are nitpicks, but it'd be great to get this as close to 'production-ready' as possible

super.onConfigurationChanged(newConfig)
if (getScreenRotation() != initialRotation) {
Timber.d("recreating activity: orientation changed with 'Change Note Type' open")
requireAnkiActivity().recreate()
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is necessary after 46cc209

Index: AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeDialog.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeDialog.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeDialog.kt	(revision 9c3be7f12d11c2b4f570cdcb4df0a86941efe9f7)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeDialog.kt	(date 1764830377708)
@@ -19,7 +19,6 @@
 
 import android.app.Dialog
 import android.content.Context
-import android.content.res.Configuration
 import android.graphics.Color
 import android.os.Bundle
 import android.text.SpannableStringBuilder
@@ -106,22 +105,11 @@
 class ChangeNoteTypeDialog : AnalyticsDialogFragment() {
     private val viewModel: ChangeNoteTypeViewModel by viewModels { defaultViewModelProviderFactory }
 
-    private var initialRotation: Int = 0
-
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
-        this.initialRotation = getScreenRotation()
         setupFlows()
     }
 
-    override fun onConfigurationChanged(newConfig: Configuration) {
-        super.onConfigurationChanged(newConfig)
-        if (getScreenRotation() != initialRotation) {
-            Timber.d("recreating activity: orientation changed with 'Change Note Type' open")
-            requireAnkiActivity().recreate()
-        }
-    }
-
     override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
         val binding = ChangeNoteTypeDialogBinding.inflate(LayoutInflater.from(requireContext()))
 
@@ -291,8 +279,6 @@
             tab.customView = binding.root
         }
 
-    private fun getScreenRotation() = ContextCompat.getDisplayOrDefault(requireContext()).rotation
-
     /** Display model for note types in the spinner */
     private data class DisplayNoteType(
         val name: String,

// unchanged after init { }
// ************************************************

// This should be lateinit, but isn't due to NotetypeJson being `value class`
Copy link
Member

Choose a reason for hiding this comment

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

Maybe... conversionTypeFlow may be a problematic change

Index: AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeViewModel.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeViewModel.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeViewModel.kt	(revision 9c3be7f12d11c2b4f570cdcb4df0a86941efe9f7)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeViewModel.kt	(date 1764830853644)
@@ -50,6 +50,7 @@
 import kotlinx.coroutines.flow.transform
 import kotlinx.coroutines.launch
 import timber.log.Timber
+import kotlin.properties.Delegates.notNull
 
 private typealias TemplateIndex = Int
 private typealias FieldIndex = Int
@@ -93,12 +94,9 @@
     // unchanged after init { }
     // ************************************************
 
-    // This should be lateinit, but isn't due to NotetypeJson being `value class`
-    private var _inputNoteType: NotetypeJson? = null
-
     /** The note type of the notes to be modified */
-    val inputNoteType: NotetypeJson
-        get() = _inputNoteType!!
+    var inputNoteType by notNull<NotetypeJson>()
+        private set
 
     /**
      * The note types which a user can provide to [setOutputNoteTypeId]
@@ -143,8 +141,7 @@
     val conversionTypeFlow: StateFlow<ConversionType> by lazy {
         outputNoteTypeFlow
             .transform { newNoteType ->
-                val source = _inputNoteType ?: return@transform
-                emit(ConversionType.fromNoteTypeChange(current = source, new = newNoteType))
+                emit(ConversionType.fromNoteTypeChange(current = inputNoteType, new = newNoteType))
             }.stateIn(
                 scope = viewModelScope,
                 started = SharingStarted.Eagerly,
@@ -231,7 +228,7 @@
 
     init {
         delayedInit {
-            _inputNoteType = withCol { getNote(noteIds.first()) }.notetype
+            inputNoteType = withCol { getNote(noteIds.first()) }.notetype
             availableNoteTypes = withCol { notetypes.all().sortedWith(NamedJSONComparator.INSTANCE) }
 
             // delayed init of outputNoteType and dependent properties

Copilot AI review requested due to automatic review settings December 4, 2025 20:59
@Haz3-jolt Haz3-jolt requested review from david-allison and removed request for Copilot December 4, 2025 21:00
Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

Tab colors are the only blocker, they aren't consistent with the rest of the dialog

Image

251382bd06b6ce4414395c9ade3cfbd4f0414e64, API36 emulator

As this is 'development only', good to go.

If tab colours are fixed, I believe this is production-ready and we can get this into 2.24

GREAT JOB!!!!!!!

@david-allison david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Dec 5, 2025
Copilot AI review requested due to automatic review settings December 5, 2025 20:31
@Haz3-jolt
Copy link
Member Author

Fixed!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new "Change Note Type" dialog feature that allows users to bulk-change the note type of multiple notes, with field and template mapping capabilities. The implementation includes a comprehensive ViewModel, Dialog UI with tabs for field and template mapping, and integration with the Card Browser. This feature is currently gated behind a developer preference flag as it's work-in-progress.

Key changes:

  • New ChangeNoteTypeViewModel and ChangeNoteTypeDialog for managing note type conversions with field/template mapping
  • Integration with Card Browser via menu item and keyboard shortcut (Ctrl+Shift+M)
  • Comprehensive test coverage with 20+ test cases covering various conversion scenarios (regular↔cloze, field/template mapping)

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeViewModel.kt Core ViewModel implementing note type change logic with field/template mapping
AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ChangeNoteTypeDialog.kt Dialog UI with tabs for field and template selection
AnkiDroid/src/test/java/com/ichi2/anki/dialogs/ChangeNoteTypeViewModelTest.kt Comprehensive test suite covering conversion scenarios
AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt Added requestChangeNoteType() and validation logic
AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt Menu integration and keyboard shortcut for change note type
AnkiDroid/src/main/java/com/ichi2/anki/utils/ViewModelUtils.kt New ViewModelDelayedInitializer interface for async ViewModel initialization
AnkiDroid/src/main/res/layout/*.xml Three new layouts for the dialog (main, fields tab, templates tab)
libanki/testutils/src/main/java/com/ichi2/anki/libanki/testutils/AnkiTest.kt Added test utilities for creating cloze note types and notes with template counts
libanki/src/main/java/com/ichi2/anki/libanki/Notetypes.kt Deprecated change() method with reference to new dialog
AnkiDroid/src/main/java/com/ichi2/utils/*.kt New utility functions for bold lists and list separators
common/src/main/java/com/ichi2/anki/common/json/JSONContainer.kt Added size property and indices property for convenience

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Haz3-jolt Haz3-jolt force-pushed the changeNT branch 2 times, most recently from 1f914bc to b8a40cd Compare December 5, 2025 20:56
Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

As discussed, I feel this can go live immediately once we cut for 2.24

This dialog allows the bulk remapping of either
fields or card templates to a different note type

A full sync is required for this operation

inputs:
* output note type
* a map of fields (based on the output)
* a map of templates (based on the output)
  * only if both input and output are non-cloze

Fixes 14134
@Haz3-jolt
Copy link
Member Author

Nothing much, just a commit message fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Second Approval Has one approval, one more approval to merge Next version Changes to be merged in the next version, to keep the current release stable. Strings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add: Change Note Types Dialog

3 participants