Change Notetype UI Rework (#1499)

* Enable access to old notetype name

* Set minimum height for ChangeNotetypeDialog

* Add bootstrap icons to change-notetype

* Move alert up and make it collapsible

* Tweak some CSS

- Add variables --sticky-bg and --sticky-border to StickyContainer
- Tweak base.css

* Add translatable string "(Nothing)"

* Rework ChangeNotetype screen

* Initially load option at newIndex and remaining options on focus

Optimization for big notetypes:
Should increase efficiency from O(n²) to O(n). Test on notetype with 500 templates shows significant improvement in load time (~10s down to ~1s).

* Try to satisfy rust test

* Change arrow direction depending on reading direction

+ add 0.5em top padding to main

* Create Alert.svelte

* Introduce CSS variable --pane-bg

* Revert "Initially load option at newIndex and remaining options on focus"

This reverts commit f42beee45c.

* Final cleanup

* Refine padding/gutter

ftl/core/change-notetype.ftl

@@ -1,5 +1,8 @@
 change-notetype-current = Current
 change-notetype-new = New
+change-notetype-nothing = (Nothing)
+change-notetype-collapse = Collapse
+change-notetype-expand = Expand
 change-notetype-will-discard-content = Will discard content on the following fields:
 change-notetype-will-discard-cards = Will remove the following cards:
 change-notetype-fields = Fields

proto/anki/notetypes.proto

@@ -167,6 +167,7 @@ message ChangeNotetypeRequest {
   int64 old_notetype_id = 4;
   int64 new_notetype_id = 5;
   int64 current_schema = 6;
+  string old_notetype_name = 7;
 }
 
 message ChangeNotetypeInfo {
@@ -175,4 +176,5 @@ message ChangeNotetypeInfo {
   repeated string new_field_names = 3;
   repeated string new_template_names = 4;
   ChangeNotetypeRequest input = 5;
+  string old_notetype_name = 6;
 }

pylib/anki/models.py

@@ -415,6 +415,7 @@ and notes.mid = ? and cards.ord = ?""",
                 note_ids=nids,
                 new_fields=field_map,
                 new_templates=template_map,
+                old_notetype_name=notetype["name"],
                 old_notetype_id=notetype["id"],
                 new_notetype_id=newModel["id"],
                 current_schema=self.col.db.scalar("select scm from col"),

qt/aqt/changenotetype.py

@@ -45,7 +45,7 @@ class ChangeNotetypeDialog(QDialog):
     def _setup_ui(self, notetype_id: NotetypeId) -> None:
         self.setWindowModality(Qt.WindowModality.ApplicationModal)
         self.mw.garbage_collect_on_dialog_finish(self)
-        self.setMinimumWidth(400)
+        self.setMinimumSize(400, 300)
         disable_help_button(self)
         restoreGeom(self, self.TITLE)
         addCloseShortcut(self)

rslib/src/backend/notetypes.rs

@@ -190,6 +190,7 @@ impl From<pb::Notetype> for Notetype {
 impl From<NotetypeChangeInfo> for pb::ChangeNotetypeInfo {
     fn from(i: NotetypeChangeInfo) -> Self {
         pb::ChangeNotetypeInfo {
+            old_notetype_name: i.old_notetype_name,
             old_field_names: i.old_field_names,
             old_template_names: i.old_template_names,
             new_field_names: i.new_field_names,
@@ -204,6 +205,7 @@ impl From<pb::ChangeNotetypeRequest> for ChangeNotetypeInput {
         ChangeNotetypeInput {
             current_schema: i.current_schema.into(),
             note_ids: i.note_ids.into_newtype(NoteId),
+            old_notetype_name: i.old_notetype_name,
             old_notetype_id: i.old_notetype_id.into(),
             new_notetype_id: i.new_notetype_id.into(),
             new_fields: i
@@ -232,6 +234,7 @@ impl From<ChangeNotetypeInput> for pb::ChangeNotetypeRequest {
         pb::ChangeNotetypeRequest {
             current_schema: i.current_schema.into(),
             note_ids: i.note_ids.into_iter().map(Into::into).collect(),
+            old_notetype_name: i.old_notetype_name,
             old_notetype_id: i.old_notetype_id.into(),
             new_notetype_id: i.new_notetype_id.into(),
             new_fields: i

rslib/src/notetype/notetypechange.rs

@@ -16,6 +16,7 @@ use crate::{
 pub struct ChangeNotetypeInput {
     pub current_schema: TimestampMillis,
     pub note_ids: Vec<NoteId>,
+    pub old_notetype_name: String,
     pub old_notetype_id: NotetypeId,
     pub new_notetype_id: NotetypeId,
     pub new_fields: Vec<Option<usize>>,
@@ -25,6 +26,7 @@ pub struct ChangeNotetypeInput {
 #[derive(Debug)]
 pub struct NotetypeChangeInfo {
     pub input: ChangeNotetypeInput,
+    pub old_notetype_name: String,
     pub old_field_names: Vec<String>,
     pub old_template_names: Vec<String>,
     pub new_field_names: Vec<String>,
@@ -77,18 +79,20 @@ impl Collection {
             .ok_or(AnkiError::NotFound)?;
 
         let current_schema = self.storage.get_collection_timestamps()?.schema_change;
+        let old_notetype_name = &old_notetype.name;
         let new_fields = default_field_map(&old_notetype, &new_notetype);
         let new_templates = default_template_map(&old_notetype, &new_notetype);
-
         Ok(NotetypeChangeInfo {
             input: ChangeNotetypeInput {
                 current_schema,
                 note_ids: vec![],
+                old_notetype_name: old_notetype_name.clone(),
                 old_notetype_id,
                 new_notetype_id,
                 new_fields,
                 new_templates,
             },
+            old_notetype_name: old_notetype_name.clone(),
             old_field_names: old_notetype.fields.iter().map(|f| f.name.clone()).collect(),
             old_template_names: old_notetype
                 .templates

sass/_vars.scss

@@ -5,6 +5,7 @@
     --text-fg: black;
     --window-bg: #ececec;
     --frame-bg: white;
+    --pane-bg: #e5e5e5;
     --border: #aaa;
     --medium-border: #b6b6b6;
     --faint-border: #e7e7e7;
@@ -44,6 +45,7 @@
     --text-fg: white;
     --window-bg: #2f2f31;
     --frame-bg: #3a3a3a;
+    --pane-bg: #3a3a3a;
     --border: #777;
     --medium-border: #444;
     --faint-border: #29292b;

ts/change-notetype/Alert.svelte

@@ -0,0 +1,60 @@
+<!--
+Copyright: Ankitects Pty Ltd and contributors
+License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+-->
+<script lang="ts">
+    import * as tr from "../lib/ftl";
+    import Badge from "../components/Badge.svelte";
+    import { MapContext } from "./lib";
+    import { plusIcon, minusIcon } from "./icons";
+    import { slide } from "svelte/transition";
+
+    export let unused: string[];
+    export let ctx: MapContext;
+
+    let unusedMsg: string;
+    $: unusedMsg =
+        ctx === MapContext.Field
+            ? tr.changeNotetypeWillDiscardContent()
+            : tr.changeNotetypeWillDiscardCards();
+
+    let maxItems: number = 3;
+    let collapsed: boolean = true;
+    $: collapseMsg = collapsed
+        ? tr.changeNotetypeExpand()
+        : tr.changeNotetypeCollapse();
+    $: icon = collapsed ? plusIcon : minusIcon;
+</script>
+
+<div class="alert alert-warning" in:slide out:slide>
+    {#if unused.length > maxItems}
+        <div class="clickable" on:click={() => (collapsed = !collapsed)}>
+            <Badge iconSize={80}>
+                {@html icon}
+            </Badge>
+            {collapseMsg}
+        </div>
+    {/if}
+    {unusedMsg}
+    {#if collapsed}
+        <div>
+            {unused.slice(0, maxItems).join(", ")}
+            {#if unused.length > maxItems}
+                ... (+{unused.length - maxItems})
+            {/if}
+        </div>
+    {:else}
+        <ul>
+            {#each unused as entry}
+                <li>{entry}</li>
+            {/each}
+        </ul>
+    {/if}
+</div>
+
+<style lang="scss">
+    .clickable {
+        cursor: pointer;
+        font-weight: bold;
+    }
+</style>

ts/change-notetype/BUILD.bazel

@@ -2,6 +2,7 @@ load("//ts:prettier.bzl", "prettier_test")
 load("//ts:eslint.bzl", "eslint_test")
 load("//ts/svelte:svelte.bzl", "compile_svelte", "svelte_check")
 load("//ts:esbuild.bzl", "esbuild")
+load("//ts:vendor.bzl", "copy_bootstrap_icons")
 load("//ts:compile_sass.bzl", "compile_sass")
 load("//ts:typescript.bzl", "typescript")
 load("//ts:jest.bzl", "jest_test")
@@ -48,6 +49,7 @@ esbuild(
     visibility = ["//visibility:public"],
     deps = [
         ":base_css",
+        "@npm//bootstrap-icons",
         ":index",
         ":svelte",
     ],

ts/change-notetype/ChangeNotetypePage.svelte

@@ -4,31 +4,54 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
 <script lang="ts">
     import * as tr from "../lib/ftl";
-    import NotetypeSelector from "./NotetypeSelector.svelte";
-    import Mapper from "./Mapper.svelte";
-    import { ChangeNotetypeState, MapContext } from "./lib";
     import marked from "marked";
+    import { ChangeNotetypeState, MapContext } from "./lib";
+    import Container from "../components/Container.svelte";
+    import Row from "../components/Row.svelte";
+    import Col from "../components/Col.svelte";
+    import NotetypeSelector from "./NotetypeSelector.svelte";
+    import StickyNav from "./StickyNav.svelte";
+    import Mapper from "./Mapper.svelte";
+    import Spacer from "../components/Spacer.svelte";
 
     export let state: ChangeNotetypeState;
-    let info = state.info;
+    $: info = state.info;
+    let offset: number;
 </script>
 
-<NotetypeSelector {state} />
+<div bind:offsetHeight={offset}>
+    <NotetypeSelector {state} />
+    <Spacer --height="1em" />
+</div>
 
-<h5>{tr.changeNotetypeFields()}</h5>
+<div id="scrollArea" style="--offset: {offset}px; --gutter-inline: 0.25rem;">
-
+    <Row class="gx-0" --cols={2}>
-<Mapper {state} ctx={MapContext.Field} />
+        <Col --col-size={1} breakpoint="md">
-
+            <Container>
-<h5>{tr.changeNotetypeTemplates()}</h5>
+                <StickyNav {state} ctx={MapContext.Field} />
-
+                <Mapper {state} ctx={MapContext.Field} />
-{#if $info.templates}
+            </Container>
+        </Col>
+        <Col --col-size={1} breakpoint="md">
+            <Container>
+                <StickyNav {state} ctx={MapContext.Template} />
+                {#if $info.templates}
                     <Mapper {state} ctx={MapContext.Template} />
-{:else}
+                {:else}
                     <div>{@html marked(tr.changeNotetypeToFromCloze())}</div>
-{/if}
+                {/if}
+            </Container>
+        </Col>
+    </Row>
+</div>
 
 <style>
-    h5 {
+    #scrollArea {
-        margin-top: 1em;
+        padding: 0;
+        overflow: hidden auto;
+        background: var(--pane-bg);
+        height: calc(100% - var(--offset));
+        border: 1px solid var(--medium-border);
+        border-radius: 0.25rem;
     }
 </style>

ts/change-notetype/Mapper.svelte

@@ -4,43 +4,20 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
 <script lang="ts">
     import Container from "../components/Container.svelte";
-    import Row from "../components/Row.svelte";
-    import Col from "../components/Col.svelte";
     import MapperRow from "./MapperRow.svelte";
-    import * as tr from "../lib/ftl";
+    import Spacer from "../components/Spacer.svelte";
-    import { ChangeNotetypeState, MapContext } from "./lib";
+    import type { ChangeNotetypeState, MapContext } from "./lib";
-    import { slide } from "svelte/transition";
 
     export let state: ChangeNotetypeState;
     export let ctx: MapContext;
 
     let info = state.info;
-
-    $: unused = $info.unusedItems(ctx);
-    $: unusedMsg =
-        ctx === MapContext.Field
-            ? tr.changeNotetypeWillDiscardContent()
-            : tr.changeNotetypeWillDiscardCards();
 </script>
 
-<Container --gutter-inline="0.5rem" --gutter-block="0.1rem">
+<Spacer --height="0.5rem" />
-    <Row --cols={2}>
-        <Col --col-size={1}><b>{tr.changeNotetypeCurrent()}</b></Col>
-        <Col --col-size={1}><b>{tr.changeNotetypeNew()}</b></Col>
-    </Row>
 
+<Container --gutter-inline="0.5rem" --gutter-block="0.15rem">
     {#each $info.mapForContext(ctx) as _, newIndex}
         <MapperRow {state} {ctx} {newIndex} />
     {/each}
 </Container>
-
-{#if unused.length > 0}
-    <div class="alert alert-warning" in:slide out:slide>
-        {unusedMsg}
-        <ul>
-            {#each unused as entry}
-                <li>{entry}</li>
-            {/each}
-        </ul>
-    </div>
-{/if}

ts/change-notetype/NotetypeSelector.svelte

@@ -4,19 +4,21 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
 <script lang="ts">
     import type { ChangeNotetypeState } from "./lib";
-
     import StickyContainer from "../components/StickyContainer.svelte";
     import ButtonToolbar from "../components/ButtonToolbar.svelte";
     import Item from "../components/Item.svelte";
     import ButtonGroup from "../components/ButtonGroup.svelte";
     import ButtonGroupItem from "../components/ButtonGroupItem.svelte";
-
+    import LabelButton from "../components/LabelButton.svelte";
+    import Badge from "../components/Badge.svelte";
+    import { arrowRightIcon, arrowLeftIcon } from "./icons";
     import SelectButton from "../components/SelectButton.svelte";
     import SelectOption from "../components/SelectOption.svelte";
     import SaveButton from "./SaveButton.svelte";
 
     export let state: ChangeNotetypeState;
     let notetypes = state.notetypes;
+    let info = state.info;
 
     async function blur(event: Event): Promise<void> {
         await state.setTargetNotetypeIndex(
@@ -25,8 +27,24 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     }
 </script>
 
-<StickyContainer --gutter-block="0.1rem" --sticky-borders="0 0 1px">
+<StickyContainer --gutter-block="0.1rem">
     <ButtonToolbar class="justify-content-between" size={2.3} wrap={false}>
+        <Item>
+            <ButtonGroupItem>
+                <LabelButton disabled={true}>
+                    {$info.oldNotetypeName}
+                </LabelButton>
+            </ButtonGroupItem>
+        </Item>
+        <Item>
+            <Badge iconSize={70}>
+                {#if window.getComputedStyle(document.body).direction == "rtl"}
+                    {@html arrowLeftIcon}
+                {:else}
+                    {@html arrowRightIcon}
+                {/if}
+            </Badge>
+        </Item>
         <Item>
             <ButtonGroup class="flex-grow-1">
                 <ButtonGroupItem>

ts/change-notetype/StickyNav.svelte

@@ -0,0 +1,61 @@
+<!--
+Copyright: Ankitects Pty Ltd and contributors
+License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+-->
+<script lang="ts">
+    import * as tr from "../lib/ftl";
+    import Badge from "../components/Badge.svelte";
+    import Alert from "./Alert.svelte";
+    import Container from "../components/Container.svelte";
+    import Row from "../components/Row.svelte";
+    import Col from "../components/Col.svelte";
+    import { exclamationIcon } from "./icons";
+    import { ChangeNotetypeState, MapContext } from "./lib";
+    import StickyContainer from "../components/StickyContainer.svelte";
+
+    export let state: ChangeNotetypeState;
+    export let ctx: MapContext;
+
+    $: info = state.info;
+
+    let heading: string =
+        ctx === MapContext.Field
+            ? tr.changeNotetypeFields()
+            : tr.changeNotetypeTemplates();
+
+    $: unused = $info.unusedItems(ctx);
+</script>
+
+<StickyContainer
+    --sticky-bg={"var(--pane-bg)"}
+    --sticky-border="var(--border)"
+    --sticky-borders="0px 0 1px"
+>
+    <h1>
+        {heading}
+        {#if unused.length > 0}
+            <Badge iconSize={80}>
+                {@html exclamationIcon}
+            </Badge>
+        {/if}
+    </h1>
+
+    {#if unused.length > 0}
+        <Alert {unused} {ctx} />
+    {/if}
+
+    {#if $info.templates}
+        <Container --gutter-inline="0.5rem" --gutter-block="0.2rem">
+            <Row --cols={2}>
+                <Col --col-size={1}><b>{tr.changeNotetypeCurrent()}</b></Col>
+                <Col --col-size={1}><b>{tr.changeNotetypeNew()}</b></Col>
+            </Row>
+        </Container>
+    {/if}
+</StickyContainer>
+
+<style lang="scss">
+    h1 {
+        padding-top: 0.5em;
+    }
+</style>

ts/change-notetype/change-notetype-base.scss

@@ -15,10 +15,8 @@
 }
 
 body {
-    width: min(100vw, 35em);
+    width: min(100vw, 70em);
     margin: 0 auto;
-    // leave some space for rounded screens
-    margin-bottom: 2em;
 }
 
 html {
@@ -26,8 +24,8 @@ html {
 }
 
 #main {
-    padding: 0.5em;
+    padding: 0.5em 1em 1em 1em;
-    padding-top: 0;
+    height: 100vh;
 }
 
 // override the default down arrow colour in <select> elements

ts/change-notetype/icons.ts

@@ -0,0 +1,10 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+/// <reference types="../lib/image-import" />
+
+export { default as arrowRightIcon } from "bootstrap-icons/icons/arrow-right.svg";
+export { default as arrowLeftIcon } from "bootstrap-icons/icons/arrow-left.svg";
+export { default as exclamationIcon } from "bootstrap-icons/icons/exclamation-circle.svg";
+export { default as plusIcon } from "bootstrap-icons/icons/plus-lg.svg";
+export { default as minusIcon } from "bootstrap-icons/icons/dash-lg.svg";

ts/change-notetype/lib.test.ts

@@ -8,6 +8,7 @@
 import { ChangeNotetypeState, negativeOneToNull, MapContext } from "./lib";
 import { Notetypes } from "../lib/proto";
 import { get } from "svelte/store";
+import * as tr from "../lib/ftl";
 
 const exampleNames = {
     entries: [
@@ -45,6 +46,7 @@ const exampleInfoDifferent = {
         oldNotetypeId: "1623289129847",
         newNotetypeId: "1623289129849",
         currentSchema: "1623302002316",
+        oldNotetypeName: "Basic",
     },
 };
 
@@ -59,6 +61,7 @@ const exampleInfoSame = {
         oldNotetypeId: "1623289129847",
         newNotetypeId: "1623289129847",
         currentSchema: "1623302002316",
+        oldNotetypeName: "Basic",
     },
 };
 
@@ -92,7 +95,7 @@ test("mapping", () => {
     expect(get(state.info).getNewName(MapContext.Field, 1)).toBe("Back");
     expect(get(state.info).getNewName(MapContext.Field, 2)).toBe("Add Reverse");
     expect(get(state.info).getOldNamesIncludingNothing(MapContext.Field)).toStrictEqual(
-        ["Front", "Back", "(Nothing)"],
+        ["Front", "Back", tr.changeNotetypeNothing()],
     );
     expect(get(state.info).getOldIndex(MapContext.Field, 0)).toBe(0);
     expect(get(state.info).getOldIndex(MapContext.Field, 1)).toBe(1);
@@ -103,7 +106,7 @@ test("mapping", () => {
     // the same template shouldn't be mappable twice
     expect(
         get(state.info).getOldNamesIncludingNothing(MapContext.Template),
-    ).toStrictEqual(["Card 1", "(Nothing)"]);
+    ).toStrictEqual(["Card 1", tr.changeNotetypeNothing()]);
     expect(get(state.info).getOldIndex(MapContext.Template, 0)).toBe(0);
     expect(get(state.info).getOldIndex(MapContext.Template, 1)).toBe(1);
     state.setOldIndex(MapContext.Template, 1, 0);

ts/change-notetype/lib.ts

@@ -1,6 +1,7 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
+import * as tr from "../lib/ftl";
 import { Notetypes } from "../lib/proto";
 import { postRequest } from "../lib/postrequest";
 import { readable, Readable } from "svelte/store";
@@ -45,6 +46,7 @@ export function negativeOneToNull(list: number[]): (number | null)[] {
 export class ChangeNotetypeInfoWrapper {
     fields: (number | null)[];
     templates?: (number | null)[];
+    oldNotetypeName: string;
     readonly info: Notetypes.ChangeNotetypeInfo;
 
     constructor(info: Notetypes.ChangeNotetypeInfo) {
@@ -54,6 +56,7 @@ export class ChangeNotetypeInfoWrapper {
             this.templates = negativeOneToNull(templates);
         }
         this.fields = negativeOneToNull(info.input!.newFields!);
+        this.oldNotetypeName = info.oldNotetypeName;
     }
 
     /// A list with an entry for each field/template in the new notetype, with
@@ -72,7 +75,7 @@ export class ChangeNotetypeInfoWrapper {
 
     /// Return all the old names, with "Nothing" at the end.
     getOldNamesIncludingNothing(ctx: MapContext): string[] {
-        return [...this.getOldNames(ctx), "(Nothing)"];
+        return [...this.getOldNames(ctx), tr.changeNotetypeNothing()];
     }
 
     /// Old names without "Nothing" at the end.
@@ -82,6 +85,10 @@ export class ChangeNotetypeInfoWrapper {
             : this.info.oldFieldNames;
     }
 
+    getOldNotetypeName(): string {
+        return this.info.oldNotetypeName;
+    }
+
     getNewName(ctx: MapContext, idx: number): string {
         return (
             ctx == MapContext.Template
@@ -97,7 +104,6 @@ export class ChangeNotetypeInfoWrapper {
             (idx) => !usedEntries.has(idx),
         );
         const unusedNames = unusedIdxs.map((idx) => oldNames[idx]);
-        unusedNames.sort();
         return unusedNames;
     }
 

ts/components/LabelButton.svelte

@@ -52,6 +52,9 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     @use "sass/button-mixins" as button;
 
     button {
+        white-space: nowrap;
+        overflow: hidden;
+        text-overflow: ellipsis;
         padding: 0 calc(var(--buttons-size) / 3);
         font-size: var(--base-font-size);
         width: auto;

ts/components/StickyContainer.svelte

@@ -30,9 +30,9 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         right: 0;
         z-index: 10;
 
-        background: var(--window-bg);
+        background: var(--sticky-bg, var(--window-bg));
         border-style: solid;
-        border-color: var(--medium-border);
+        border-color: var(--sticky-border, var(--medium-border));
         border-width: var(--sticky-borders, 0);
     }
 </style>