From add696a94190146e9ea31eef5286a8166b284b7f Mon Sep 17 00:00:00 2001
From: Michele Ceriotti <michele.ceriotti@gmail.com>
Date: Thu, 21 Sep 2023 10:57:41 -0700
Subject: [PATCH] Fixed another UI bug in the multivalued selector

Also more informative name for the HTMLOptions machinery with multivalued options
---
 python/examples/shapes.py |  7 ++++++-
 src/options.ts            | 12 +++++++++---
 src/structure/options.ts  |  5 ++---
 src/structure/viewer.ts   |  5 +++--
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/python/examples/shapes.py b/python/examples/shapes.py
index aa23b5352..e481e0146 100644
--- a/python/examples/shapes.py
+++ b/python/examples/shapes.py
@@ -10,6 +10,7 @@
 Note that the same parameters can be used with `chemiscope.show`
 to visualize an interactive widget in a Jupyter notebook.
 """
+
 import ase.io as aseio
 import chemiscope
 import numpy as np
@@ -127,7 +128,11 @@
         },
         "structure": [  # structure positioning is relative to the origin of the axes
             {"position": [0, 4, 0], "color": 0xFF0000},
-            {"position": [3, 2, 1], "color": 0x00FF00},
+            {
+                "position": [3, 2, 1],
+                "color": 0x00FF00,
+                "orientation": [0.2, 0.4, 0.1, 1],
+            },
         ],
     },
 )
diff --git a/src/options.ts b/src/options.ts
index f4f86d01b..15ed5d45c 100644
--- a/src/options.ts
+++ b/src/options.ts
@@ -12,7 +12,7 @@ import { Settings } from './dataset';
  * Possible HTML attributes to attach to a setting
  */
 // this is mostly to catch typo early. Feel free to add more!
-type Attribute = 'value' | 'checked' | 'innerText' | 'options';
+type Attribute = 'value' | 'checked' | 'innerText' | 'multival';
 
 /// Type mapping for options
 interface OptionsTypeMap {
@@ -146,7 +146,7 @@ export class HTMLOption<T extends OptionsType> {
      */
     public changed(origin: OptionModificationOrigin) {
         for (const bound of this._boundList) {
-            if (bound.attribute === 'options') {
+            if (bound.attribute === 'multival') {
                 // options take a list of comma-separated values to allow multiple settings
                 const values = (this._value as string).split(',');
                 const element = bound.element as HTMLSelectElement;
@@ -185,7 +185,7 @@ export class HTMLOption<T extends OptionsType> {
         element = element as HTMLElement;
 
         let listener: (event: Event) => void;
-        if (attribute === 'options') {
+        if (attribute === 'multival') {
             listener = (event: Event) => {
                 // we need a special handler for multi-select options
                 assert(event.target !== null);
@@ -196,6 +196,12 @@ export class HTMLOption<T extends OptionsType> {
 
                 this._update(values.toString(), 'DOM');
             };
+            // also initializes the state of the option list
+            const values = (this._value as string).split(',');
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
+            for (const option of (element as any).options) {
+                option.selected = values.includes(option.value);
+            }
         } else {
             listener = (event: Event) => {
                 assert(event.target !== null);
diff --git a/src/structure/options.ts b/src/structure/options.ts
index d6023e0a5..8e34da97f 100644
--- a/src/structure/options.ts
+++ b/src/structure/options.ts
@@ -187,7 +187,7 @@ export class StructureOptions extends OptionsGroup {
         // Position modal near the actual viewer
         openModal.addEventListener('click', () => {
             // only set style once, on first open, and keep previous position
-            // on next open to keep the 'draged-to' position
+            // on next open to keep the 'dragged-to' position
             if (modalDialog.getAttribute('data-initial-modal-positions-set') === null) {
                 modalDialog.setAttribute('data-initial-modal-positions-set', 'true');
 
@@ -206,7 +206,6 @@ export class StructureOptions extends OptionsGroup {
                 modalDialog.style.top = `${top}px`;
                 modalDialog.style.left = `${left}px`;
             }
-
             modal.open();
         });
 
@@ -228,7 +227,7 @@ export class StructureOptions extends OptionsGroup {
         this.atomLabels.bind(this.getModalElement('atom-labels'), 'checked');
 
         const selectShape = this.getModalElement<HTMLSelectElement>('shapes');
-        this.shape.bind(selectShape, 'options');
+        this.shape.bind(selectShape, 'multival');
 
         this.spaceFilling.bind(this.getModalElement('space-filling'), 'checked');
         this.bonds.bind(this.getModalElement('bonds'), 'checked');
diff --git a/src/structure/viewer.ts b/src/structure/viewer.ts
index a6d3f70e8..c2b889524 100644
--- a/src/structure/viewer.ts
+++ b/src/structure/viewer.ts
@@ -432,7 +432,6 @@ export class MoleculeViewer {
             this._styles.noShape.replaceSync('.chsp-hide-if-no-shapes { display: none; }');
         } else {
             this._styles.noShape.replaceSync('');
-
             const selectShape = this._options.getModalElement<HTMLSelectElement>('shapes');
             selectShape.options.length = 0;
             for (const key of Object.keys(structure['shapes'])) {
@@ -443,7 +442,9 @@ export class MoleculeViewer {
             // will be accessible with a scrollbar
             selectShape.size = Math.min(Object.keys(structure['shapes']).length, 3);
 
-            this._options.shape.bind(selectShape, 'options');
+            // set the highlighting to reflect the options
+
+            this._options.shape.bind(selectShape, 'multival');
         }
 
         this._updateStyle();