Skip to content

Commit

Permalink
Fixed another UI bug in the multivalued selector
Browse files Browse the repository at this point in the history
Also more informative name for the HTMLOptions machinery with multivalued options
  • Loading branch information
ceriottm committed Sep 21, 2023
1 parent abe83ed commit add696a
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 9 deletions.
7 changes: 6 additions & 1 deletion python/examples/shapes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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],
},
],
},
)
Expand Down
12 changes: 9 additions & 3 deletions src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);

Check failure on line 203 in src/options.ts

View workflow job for this annotation

GitHub Actions / npm-test (18.x)

Unsafe member access .selected on an `any` value

Check failure on line 203 in src/options.ts

View workflow job for this annotation

GitHub Actions / npm-test (18.x)

Unsafe argument of type `any` assigned to a parameter of type `string`

Check failure on line 203 in src/options.ts

View workflow job for this annotation

GitHub Actions / npm-test (18.x)

Unsafe member access .value on an `any` value

Check failure on line 203 in src/options.ts

View workflow job for this annotation

GitHub Actions / npm-test (20.x)

Unsafe member access .selected on an `any` value

Check failure on line 203 in src/options.ts

View workflow job for this annotation

GitHub Actions / npm-test (20.x)

Unsafe argument of type `any` assigned to a parameter of type `string`

Check failure on line 203 in src/options.ts

View workflow job for this annotation

GitHub Actions / npm-test (20.x)

Unsafe member access .value on an `any` value
}
} else {
listener = (event: Event) => {
assert(event.target !== null);
Expand Down
5 changes: 2 additions & 3 deletions src/structure/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -206,7 +206,6 @@ export class StructureOptions extends OptionsGroup {
modalDialog.style.top = `${top}px`;
modalDialog.style.left = `${left}px`;
}

modal.open();
});

Expand All @@ -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');
Expand Down
5 changes: 3 additions & 2 deletions src/structure/viewer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'])) {
Expand All @@ -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();
Expand Down

0 comments on commit add696a

Please sign in to comment.