Skip to content

Commit

Permalink
WIP: ui: Enable mixed-state behavior for master checkbox in AdavncedM…
Browse files Browse the repository at this point in the history
…ultiSelect

The AdvancedMultiSelect should adhere to some set of human interface
guidelines. In the absence of a formal, agreed upon set of guidelines
for Infection Monkey, this commit uses KDE's guidelines for checkboxes:
https://hig.kde.org/components/editing/checkbox.html

When child checkboxes are not all checked, the master checkbox displays
a mixed-state icon, instead of a checked icon. Clicking the mixed-state
icon checks all child checkboxes. Clicking an unchecked master checkbox
also enables all child checkboxes.

In the past, clicking an unchecked master checkbox checked only the
*default* child checkboxes. While this may seem desirable so that unsafe
exploits do not accidentally get selected by the user, it will confuse
and frustrate users, as master/child checkboxes do not normally function
this way. If there is concern that users may unknowingly select unsafe
exploits/options, we should pop up a warning to inform the user when the
config is saved/submitted.

Issue guardicore#891
  • Loading branch information
mssalvatore committed Jan 12, 2021
1 parent 878f959 commit 5d667f5
Showing 1 changed file with 36 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from "react";
import {Card, Button, Form} from 'react-bootstrap';
import {FontAwesomeIcon} from '@fortawesome/react-fontawesome';
import {faCheckSquare} from '@fortawesome/free-solid-svg-icons';
import {faMinusSquare} from '@fortawesome/free-solid-svg-icons';
import {faSquare} from '@fortawesome/free-regular-svg-icons';
import {cloneDeep} from 'lodash';

Expand Down Expand Up @@ -37,15 +38,15 @@ function MasterCheckbox(props) {
value,
disabled,
onClick,
checkboxState
checkboxIcon
} = props;

return (
<Card.Header>
<Button key={`${title}-button`} value={value}
variant={'link'} disabled={disabled}
onClick={onClick}>
<FontAwesomeIcon icon={checkboxState ? faCheckSquare : faSquare}/>
<FontAwesomeIcon icon={checkboxIcon}/>
</Button>
<span className={'header-title'}>{title}</span>
</Card.Header>
Expand Down Expand Up @@ -77,42 +78,61 @@ function ChildCheckbox(props) {
class AdvancedMultiSelect extends React.Component {
constructor(props) {
super(props)
this.state = {masterCheckbox: true, infoPaneParams: getDefaultPaneParams(props.schema.items.$ref, props.registry)};
this.state = {
masterCheckboxIcon: this.getMasterCheckboxIconState(props.value),
infoPaneParams: getDefaultPaneParams(props.schema.items.$ref, props.registry)
};
this.onMasterCheckboxClick = this.onMasterCheckboxClick.bind(this);
this.onChildCheckboxClick = this.onChildCheckboxClick.bind(this);
this.setPaneInfo = this.setPaneInfo.bind(this, props.schema.items.$ref, props.registry);
}

onMasterCheckboxClick() {
if (this.state.masterCheckbox) {
this.props.onChange([]);
} else {
this.props.onChange(this.props.schema.default);
var newValues = this.props.options.enumOptions.map(({value}) => value);

if (this.state.masterCheckboxIcon == faCheckSquare) {
newValues = [];
}

this.toggleMasterCheckbox();
this.props.onChange(newValues);
this.setMasterCheckboxIconState(newValues);
}

onChildCheckboxClick(value) {
this.props.onChange(this.getSelectValuesAfterClick(value));
var selectValues = this.getSelectValuesAfterClick(value)
this.props.onChange(selectValues);

this.setMasterCheckboxIconState(selectValues);
}

getSelectValuesAfterClick(clickedValue) {
const valueArray = cloneDeep(this.props.value);

if (valueArray.includes(clickedValue)) {
return valueArray.filter((e) => {
return e !== clickedValue;
});
return valueArray.filter(e => e !== clickedValue);
} else {
valueArray.push(clickedValue);
return valueArray;
}
}

toggleMasterCheckbox() {
this.setState((state) => ({
masterCheckbox: !state.masterCheckbox
// TODO: Use an enum for master checkbox state, rather than putting the icon.
// Let MasterCheckbox decide how to display itself. The parent component
// should just tell it what state it's in.
getMasterCheckboxIconState(selectValues) {
var newCheckboxIcon = faCheckSquare;

if (selectValues.length == 0)
newCheckboxIcon = faSquare;
else if (selectValues.length != this.props.options.enumOptions.length)
newCheckboxIcon = faMinusSquare;

return newCheckboxIcon;
}

setMasterCheckboxIconState(selectValues) {
this.setState(() => ({
masterCheckboxIcon: this.getMasterCheckboxIconState(selectValues)
}));
}

Expand All @@ -132,7 +152,6 @@ class AdvancedMultiSelect extends React.Component {
readonly,
multiple,
autofocus,
onChange,
registry
} = this.props;

Expand All @@ -143,7 +162,7 @@ class AdvancedMultiSelect extends React.Component {
<div className={'advanced-multi-select'}>
<MasterCheckbox title={schema.title} value={value}
disabled={disabled} onClick={this.onMasterCheckboxClick}
checkboxState={this.state.masterCheckbox}/>
checkboxIcon={this.state.masterCheckboxIcon}/>
<Form.Group
style={{height: `${getComponentHeight(enumOptions.length)}px`}}
id={id}
Expand Down

0 comments on commit 5d667f5

Please sign in to comment.