From 19bc09196f22e6c1f65e3fd9190c894c4d621a20 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 11 Jan 2021 20:24:50 -0500 Subject: [PATCH] ui: Enable mixed-state behavior for master checkbox in AdavncedMultiSelect 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 #891 --- .../ui-components/AdvancedMultiSelect.js | 60 ++++++++++++++----- 1 file changed, 44 insertions(+), 16 deletions(-) diff --git a/monkey/monkey_island/cc/ui/src/components/ui-components/AdvancedMultiSelect.js b/monkey/monkey_island/cc/ui/src/components/ui-components/AdvancedMultiSelect.js index 7507f234f..db759b013 100644 --- a/monkey/monkey_island/cc/ui/src/components/ui-components/AdvancedMultiSelect.js +++ b/monkey/monkey_island/cc/ui/src/components/ui-components/AdvancedMultiSelect.js @@ -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'; @@ -9,6 +10,12 @@ import {getComponentHeight} from './utils/HeightCalculator'; import {resolveObjectPath} from './utils/ObjectPathResolver'; import InfoPane from './InfoPane'; +const MasterCheckboxState = { + NONE: 0, + MIXED: 1, + ALL: 2 +} + // Definitions passed to components only contains value and label, // custom fields like "info" or "links" must be pulled from registry object using this function @@ -40,12 +47,19 @@ function MasterCheckbox(props) { checkboxState } = props; + var newCheckboxIcon = faCheckSquare; + + if (checkboxState == MasterCheckboxState.NONE) + newCheckboxIcon = faSquare; + else if (checkboxState == MasterCheckboxState.MIXED) + newCheckboxIcon = faMinusSquare; + return ( {title} @@ -77,42 +91,57 @@ 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 = { + masterCheckboxState: this.getMasterCheckboxState(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.masterCheckboxState == MasterCheckboxState.ALL) { + newValues = []; } - this.toggleMasterCheckbox(); + this.props.onChange(newValues); + this.setMasterCheckboxState(newValues); } onChildCheckboxClick(value) { - this.props.onChange(this.getSelectValuesAfterClick(value)); + var selectValues = this.getSelectValuesAfterClick(value) + this.props.onChange(selectValues); + + this.setMasterCheckboxState(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 + getMasterCheckboxState(selectValues) { + if (selectValues.length == 0) + return MasterCheckboxState.NONE; + + if (selectValues.length != this.props.options.enumOptions.length) + return MasterCheckboxState.MIXED; + + return MasterCheckboxState.ALL; + } + + setMasterCheckboxState(selectValues) { + this.setState(() => ({ + masterCheckboxState: this.getMasterCheckboxState(selectValues) })); } @@ -132,7 +161,6 @@ class AdvancedMultiSelect extends React.Component { readonly, multiple, autofocus, - onChange, registry } = this.props; @@ -143,7 +171,7 @@ class AdvancedMultiSelect extends React.Component {
+ checkboxState={this.state.masterCheckboxState}/>