From 214ab9baa9a9fa7310f8fb08534b9f49c353efa8 Mon Sep 17 00:00:00 2001 From: Amir Golparvar <amir.golparvar@physik.hu-berlin.de> Date: Thu, 25 Jul 2024 06:20:31 +0000 Subject: [PATCH] Resolve "BoolEditQuantity looks True when None" --- .../editQuantity/BoolEditQuantity.js | 76 ++++++++--- .../editQuantity/BoolEditQuantity.spec.js | 129 ++++++++++++++++++ .../editQuantity/EditQuantityExamples.js | 17 +++ 3 files changed, 204 insertions(+), 18 deletions(-) create mode 100644 gui/src/components/editQuantity/BoolEditQuantity.spec.js diff --git a/gui/src/components/editQuantity/BoolEditQuantity.js b/gui/src/components/editQuantity/BoolEditQuantity.js index d138234b68..22fa5d5147 100644 --- a/gui/src/components/editQuantity/BoolEditQuantity.js +++ b/gui/src/components/editQuantity/BoolEditQuantity.js @@ -18,7 +18,7 @@ import React, {useCallback} from 'react' import { FormControlLabel, - Checkbox + Checkbox, FormLabel, RadioGroup, Radio, FormControl } from '@material-ui/core' import PropTypes from 'prop-types' import {getFieldProps, WithHelp} from './StringEditQuantity' @@ -27,33 +27,73 @@ import {useRecoilValue} from "recoil" import {configState} from "../archive/ArchiveBrowser" export const BoolEditQuantity = React.memo(function BoolEditQuantity(props) { - const {quantityDef, value, onChange, ...otherProps} = props + const {quantityDef, value, onChange, booleanLabels, ...otherProps} = props const fieldProps = getFieldProps(quantityDef) const config = useRecoilValue(configState) const label = getDisplayLabel(quantityDef, true, config?.showMeta) - const handleChange = useCallback((event) => { - const value = event.target.checked + const handleCheckboxChange = useCallback(e => { + const value = e.target.checked if (onChange) { onChange(value) } }, [onChange]) - return <WithHelp {...fieldProps} label={label}> - <FormControlLabel - control={( - <Checkbox - onChange={handleChange} - color="primary" - checked={!!value} - {...otherProps} - />)} - label={label} - /> - </WithHelp> + const handleRadioChange = useCallback(e => { + const value = e.target.value === 'true' ? true : (e.target.value === 'false' ? false : undefined) + if (onChange) { + onChange(value) + } + }, [onChange]) + + return ( + <WithHelp {...fieldProps} label={label}> + {booleanLabels ? ( + <FormControl> + <FormLabel>{label}</FormLabel> + <RadioGroup row value={String(value)} onChange={handleRadioChange}> + {Object.keys(booleanLabels).map(key => ( + <FormControlLabel + value={key} + key={key} + control={<Radio {...otherProps} />} + label={booleanLabels[key]} + /> + ))} + </RadioGroup> + </FormControl> + ) : ( + <FormControlLabel + control={ + <Checkbox + onChange={handleCheckboxChange} + color="primary" + checked={value === true} + indeterminate={value === undefined} + {...otherProps} + /> + } + label={label} + /> + )} + </WithHelp> + ) }) + BoolEditQuantity.propTypes = { quantityDef: PropTypes.object.isRequired, - value: PropTypes.bool, - onChange: PropTypes.func + value: PropTypes.oneOf([true, false, undefined]), + onChange: PropTypes.func, + booleanLabels: (props, propName, componentName) => { + const labels = props[propName] + if (!labels) return null + if (typeof labels !== 'object') { + return new Error(`Invalid prop \`${propName}\` supplied to \`${componentName}\`. Expected an object.`) + } + const validKeys = ['true', 'false', 'undefined'] + if (!Object.keys(labels).every(key => validKeys.includes(key))) { + return new Error(`Invalid prop \`${propName}\` supplied to \`${componentName}\`. Expected keys "true", "false", "undefined".`) + } + return null + } } diff --git a/gui/src/components/editQuantity/BoolEditQuantity.spec.js b/gui/src/components/editQuantity/BoolEditQuantity.spec.js new file mode 100644 index 0000000000..599edd23e3 --- /dev/null +++ b/gui/src/components/editQuantity/BoolEditQuantity.spec.js @@ -0,0 +1,129 @@ +/* + * Copyright The NOMAD Authors. + * + * This file is part of NOMAD. See https://nomad-lab.eu for further info. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react' +import {renderNoAPI, screen} from '../conftest.spec' +import {BoolEditQuantity} from './BoolEditQuantity' +import {fireEvent} from "@testing-library/react" +import {act} from 'react-dom/test-utils' +import userEvent from '@testing-library/user-event' + +const handleChange = jest.fn() +const quantityDef = { + name: 'name', + description: `This is **MARKDOWN** help text.` +} +const booleanLabels = {true: 'On', false: 'Off', undefined: 'Unknown'} + +const getRadioElements = () => { + const radioOn = screen.getByLabelText(booleanLabels[true]) + const radioOff = screen.getByLabelText(booleanLabels[false]) + const radioUnknown = screen.getByLabelText(booleanLabels[undefined]) + return {radioOn, radioOff, radioUnknown} +} + +test('checkbox initial value is displayed correctly', async () => { + renderNoAPI(<BoolEditQuantity + quantityDef={quantityDef} + onChange={handleChange} + value={true} + />) + const checkbox = screen.getByRole('checkbox') + expect(checkbox.checked).toBe(true) + expect(checkbox.indeterminate).toBe(false) +}) + +test('radio buttons are displayed correctly with initial value', async () => { + renderNoAPI(<BoolEditQuantity + quantityDef={quantityDef} + onChange={handleChange} + value={true} + booleanLabels={booleanLabels} + />) + const {radioOn, radioOff, radioUnknown} = getRadioElements() + expect(radioOn).toBeInTheDocument() + expect(radioOff).toBeInTheDocument() + expect(radioUnknown).toBeInTheDocument() + expect(radioOn.checked).toBe(true) + expect(radioOff.checked).toBe(false) + expect(radioUnknown.checked).toBe(false) +}) + +test('external change (without internal change) updates correctly for checkbox', async () => { + const {rerender} = renderNoAPI(<BoolEditQuantity + quantityDef={quantityDef} + onChange={handleChange} + value={false} + />) + rerender(<BoolEditQuantity + quantityDef={quantityDef} + onChange={handleChange} + value={true} + />) + const checkbox = screen.getByRole('checkbox') + expect(checkbox.checked).toBe(true) + expect(checkbox.indeterminate).toBe(false) +}) + +test('external change (without internal change) updates correctly for radio buttons', async () => { + const {rerender} = renderNoAPI(<BoolEditQuantity + quantityDef={quantityDef} + onChange={handleChange} + value={false} + booleanLabels={booleanLabels} + />) + rerender(<BoolEditQuantity + quantityDef={quantityDef} + onChange={handleChange} + value={true} + booleanLabels={booleanLabels} + />) + const {radioOn, radioOff, radioUnknown} = getRadioElements() + expect(radioOn.checked).toBe(true) + expect(radioOff.checked).toBe(false) + expect(radioUnknown.checked).toBe(false) +}) + +test('handleChange gets called with correct arguments when user clicks on checkbox', async () => { + renderNoAPI(<BoolEditQuantity + quantityDef={quantityDef} + onChange={handleChange} + value={false} + />) + const checkbox = screen.getByRole('checkbox') + fireEvent.click(checkbox) + expect(handleChange).toHaveBeenCalledWith(true) +}) + +test('handleChange gets called with correct arguments when user clicks on radio buttons', async () => { + renderNoAPI(<BoolEditQuantity + quantityDef={quantityDef} + onChange={handleChange} + value={false} + booleanLabels={booleanLabels} + />) + const {radioOn, radioUnknown} = getRadioElements() + await act(async () => { + await userEvent.click(radioOn) + }) + expect(handleChange).toHaveBeenCalledWith(true) + await act(async () => { + await userEvent.click(radioUnknown) + }) + expect(handleChange).toHaveBeenCalledWith(undefined) +}) diff --git a/gui/src/components/editQuantity/EditQuantityExamples.js b/gui/src/components/editQuantity/EditQuantityExamples.js index 9a0b730313..edebecb842 100644 --- a/gui/src/components/editQuantity/EditQuantityExamples.js +++ b/gui/src/components/editQuantity/EditQuantityExamples.js @@ -247,6 +247,23 @@ export function EditQuantityExamples() { <BoolEditQuantity {...createDefaultProps('bool')} /> </Example> </Grid> + <Grid item> + <Example + code={` + bool_with_labels: + type: bool + m_annotations: + eln: + component: BoolEditQuantity + boolean_labels: + true: on + false: off + undefined: undecided`} + > + <BoolEditQuantity {...createDefaultProps('bool_with_labels')} + booleanLabels={{true: 'on', false: 'off', undefined: 'undecided'}}/> + </Example> + </Grid> <Grid item> <Example code={` -- GitLab