From e059aab1844e7ff04e16912e16cd7d3c9beaf7ad Mon Sep 17 00:00:00 2001
From: Matthias Metelka <62722460+kleinerpirat@users.noreply.github.com>
Date: Wed, 7 Dec 2022 06:31:37 +0100
Subject: [PATCH] Improve Select component and add it back to Change Notetype
 screen (#2239)

* Do not include oldIdx in Select change event

I included it due to confusion about the variable names in the Change Notetype components.

* Remove redundant on:change listener from NotetypeSelector

* Use Select component in Change Notetype MapperRow (again)

* Remove redundant --cols and --col-size definitions

Bootstrap divides rows into columns of equal width by default.

* Add highlight to active DropdownItem

* Remove bootstrap dropdown item styling

* Fix JS error on dropdown accept action

cause: When closing the dropdown, buttonRef was removed before the callback in setTimeout was run.
---
 ts/change-notetype/MapperRow.svelte        | 27 ++++++++++----------
 ts/change-notetype/NotetypeSelector.svelte |  6 ++---
 ts/components/DropdownItem.svelte          | 29 +++++++++++++++++-----
 ts/components/Select.svelte                |  7 ++++--
 ts/components/SelectOption.svelte          |  6 ++++-
 ts/deck-options/ConfigSelector.svelte      |  2 +-
 ts/deck-options/deck-options-base.scss     |  1 -
 ts/editor/editor-base.scss                 |  1 -
 8 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/ts/change-notetype/MapperRow.svelte b/ts/change-notetype/MapperRow.svelte
index 9afe323ad..d663fd7e0 100644
--- a/ts/change-notetype/MapperRow.svelte
+++ b/ts/change-notetype/MapperRow.svelte
@@ -5,6 +5,8 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 <script lang="ts">
     import Col from "../components/Col.svelte";
     import Row from "../components/Row.svelte";
+    import Select from "../components/Select.svelte";
+    import SelectOption from "../components/SelectOption.svelte";
     import type { ChangeNotetypeState, MapContext } from "./lib";
 
     export let state: ChangeNotetypeState;
@@ -12,26 +14,25 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     export let newIndex: number;
 
     const info = state.info;
+    $: oldIndex = $info.getOldIndex(ctx, newIndex);
 
-    function onChange(evt: Event) {
-        const oldIdx = parseInt((evt.target as HTMLSelectElement).value, 10);
-        state.setOldIndex(ctx, newIndex, oldIdx);
+    function onChange(evt: CustomEvent) {
+        oldIndex = evt.detail.value;
+        state.setOldIndex(ctx, newIndex, oldIndex);
     }
+
+    $: label = $info.getOldNamesIncludingNothing(ctx)[oldIndex];
 </script>
 
-<Row --cols={2}>
-    <Col --col-size={1}>
-        <select
-            value={$info.getOldIndex(ctx, newIndex)}
-            class="form-select"
-            on:change={onChange}
-        >
+<Row>
+    <Col>
+        <Select value={oldIndex} {label} on:change={onChange}>
             {#each $info.getOldNamesIncludingNothing(ctx) as name, idx}
-                <option value={idx}>{name}</option>
+                <SelectOption value={idx}>{name}</SelectOption>
             {/each}
-        </select>
+        </Select>
     </Col>
-    <Col --col-size={1}>
+    <Col>
         {$info.getNewName(ctx, newIndex)}
     </Col>
 </Row>
diff --git a/ts/change-notetype/NotetypeSelector.svelte b/ts/change-notetype/NotetypeSelector.svelte
index 9c0791935..3cd85e354 100644
--- a/ts/change-notetype/NotetypeSelector.svelte
+++ b/ts/change-notetype/NotetypeSelector.svelte
@@ -20,9 +20,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     $: options = Array.from($notetypes, (notetype) => notetype.name);
     $: label = options[value];
 
-    function blur(e: CustomEvent): void {
-        state.setTargetNotetypeIndex(e.detail.newIdx);
-    }
+    $: state.setTargetNotetypeIndex(value);
 </script>
 
 <ButtonToolbar class="justify-content-between" wrap={false}>
@@ -36,7 +34,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
             {@html arrowRightIcon}
         {/if}
     </Badge>
-    <Select class="flex-grow-1" bind:value {label} on:change={blur}>
+    <Select class="flex-grow-1" bind:value {label}>
         {#each options as option, idx}
             <SelectOption value={idx}>{option}</SelectOption>
         {/each}
diff --git a/ts/components/DropdownItem.svelte b/ts/components/DropdownItem.svelte
index daadda779..ed18984e9 100644
--- a/ts/components/DropdownItem.svelte
+++ b/ts/components/DropdownItem.svelte
@@ -14,13 +14,13 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     export let active = false;
     export let disabled = false;
 
+    const rtl: boolean = window.getComputedStyle(document.body).direction == "rtl";
+
     $: if (buttonRef && active) {
-        setTimeout(() =>
-            buttonRef!.scrollIntoView({
-                behavior: "smooth",
-                block: "nearest",
-            }),
-        );
+        buttonRef!.scrollIntoView({
+            behavior: "smooth",
+            block: "nearest",
+        });
     }
 
     export let tabbable = false;
@@ -33,6 +33,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     class="dropdown-item {className}"
     class:active
     class:disabled
+    class:rtl
     title={tooltip}
     on:mouseenter
     on:focus
@@ -61,5 +62,21 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
             background: var(--highlight-bg);
             color: var(--highlight-fg);
         }
+
+        /* selection highlight */
+        &:not(.rtl) {
+            border-left: 3px solid transparent;
+        }
+        &.rtl {
+            border-right: 3px solid transparent;
+        }
+        &.active {
+            &:not(.rtl) {
+                border-left-color: var(--border-focus);
+            }
+            &.rtl {
+                border-right-color: var(--border-focus);
+            }
+        }
     }
 </style>
diff --git a/ts/components/Select.svelte b/ts/components/Select.svelte
index fbb67227d..657959dbf 100644
--- a/ts/components/Select.svelte
+++ b/ts/components/Select.svelte
@@ -4,6 +4,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
 <script lang="ts">
     import { createEventDispatcher, setContext } from "svelte";
+    import { writable } from "svelte/store";
 
     import { selectKey } from "./context-keys";
     import IconConstrain from "./IconConstrain.svelte";
@@ -23,8 +24,8 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     const dispatch = createEventDispatcher();
 
     function setValue(v: number) {
-        dispatch("change", { oldIdx: value, newIdx: v });
         value = v;
+        dispatch("change", { value });
     }
 
     export let element: HTMLElement | undefined = undefined;
@@ -44,7 +45,9 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         }
     }
 
-    setContext(selectKey, setValue);
+    const selectStore = writable({ value, setValue });
+    $: $selectStore.value = value;
+    setContext(selectKey, selectStore);
 </script>
 
 <WithFloating
diff --git a/ts/components/SelectOption.svelte b/ts/components/SelectOption.svelte
index 621f5f711..e466a9d11 100644
--- a/ts/components/SelectOption.svelte
+++ b/ts/components/SelectOption.svelte
@@ -4,6 +4,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
 <script lang="ts">
     import { getContext } from "svelte";
+    import type { Writable } from "svelte/store";
 
     import { selectKey } from "./context-keys";
     import DropdownItem from "./DropdownItem.svelte";
@@ -39,11 +40,14 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         }
     }
 
-    const setValue: Function = getContext(selectKey);
+    const selectContext: Writable<{ value: number; setValue: Function }> =
+        getContext(selectKey);
+    const setValue = $selectContext.setValue;
 </script>
 
 <DropdownItem
     {disabled}
+    active={value == $selectContext.value}
     on:click={() => setValue(value)}
     on:keydown={handleKey}
     bind:buttonRef={element}
diff --git a/ts/deck-options/ConfigSelector.svelte b/ts/deck-options/ConfigSelector.svelte
index 207a1b2d3..bdf9a3ded 100644
--- a/ts/deck-options/ConfigSelector.svelte
+++ b/ts/deck-options/ConfigSelector.svelte
@@ -30,7 +30,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     }
 
     function blur(e: CustomEvent): void {
-        state.setCurrentIndex(e.detail.newIdx);
+        state.setCurrentIndex(e.detail.value);
         dispatchPresetChange();
     }
 
diff --git a/ts/deck-options/deck-options-base.scss b/ts/deck-options/deck-options-base.scss
index b5a6fdf20..b88d78d93 100644
--- a/ts/deck-options/deck-options-base.scss
+++ b/ts/deck-options/deck-options-base.scss
@@ -3,7 +3,6 @@
 // override Bootstrap transition duration
 $carousel-transition: 0.2s;
 
-@import "bootstrap/scss/dropdown";
 @import "bootstrap/scss/buttons";
 @import "bootstrap/scss/button-group";
 @import "bootstrap/scss/transitions";
diff --git a/ts/editor/editor-base.scss b/ts/editor/editor-base.scss
index cdbe76940..3b3c9a55b 100644
--- a/ts/editor/editor-base.scss
+++ b/ts/editor/editor-base.scss
@@ -6,7 +6,6 @@
 $btn-disabled-opacity: 0.4;
 @import "bootstrap/scss/buttons";
 @import "bootstrap/scss/button-group";
-@import "bootstrap/scss/dropdown";
 @import "sass/bootstrap-tooltip";
 
 html,