mirror of
https://github.com/Unleash/unleash.git
synced 2025-07-21 13:47:39 +02:00
1-3651/single value inputs (#9859)
Adds an "add value" with popover input for single-value fields (numerical and semver operators). The implementation re-uses the popover from the multi-value constraint operators, so I've extracted it for re-use. All current input types: <img width="779" alt="image" src="https://github.com/user-attachments/assets/ad522e4d-72ba-402c-ad7c-8609ef2fb3a8" /> For the new one, opening the popover when there's a value will pre-select the value, so you can override it by typing immediately: <img width="297" alt="image" src="https://github.com/user-attachments/assets/31d18f9e-6ef9-4450-9d63-ca5034b59f19" /> Buttons look pretty identical: <img width="784" alt="image" src="https://github.com/user-attachments/assets/d96b0b0d-0cbb-4262-9ca8-4ec919cbfafb" /> ## Discussion points ### Input type I haven't set an input type anywhere on the popover yet. In theory, we could use input type "number" for numerical inputs and I think it's worth looking at that, but we don't do in the old implementation either. I've added a task for it. ### Weird esc handling This implementation uses a chip for the button/value display for the single. In almost all cases it works exactly as I'd expect, but closing the popover with esc moves your focus to the top of `body`. Unfortunately, this isn't something we can address directly (trust me, I've tried), but the good news is that this was fixed in mui v6. The current major is v7, so we probably want to update before too long, which will also fix this. More info in the MUI docs: https://mui.com/material-ui/migration/upgrade-to-v6/#chip I think that for the single value entry, losing focus on esc is a fair tradeoff because it handles swapping states etc so gracefully. For the multi-value operators, however, esc is the only way to close the popover, so losing focus when you do that is not acceptable to me. As such, I'll leave the multi-value input as a button for now instead. (It's also totally fine because the button never updates or needs to change).
This commit is contained in:
parent
cb987ac78b
commit
d44b7ac6c2
@ -0,0 +1,65 @@
|
||||
import Add from '@mui/icons-material/Add';
|
||||
import { styled } from '@mui/material';
|
||||
import { forwardRef, useImperativeHandle, useRef, useState } from 'react';
|
||||
import { ValueChip } from './ValueList';
|
||||
import { AddValuesPopover, type OnAddActions } from './AddValuesPopover';
|
||||
|
||||
const StyledChip = styled(ValueChip, {
|
||||
shouldForwardProp: (prop) => prop !== 'hasValue',
|
||||
})<{ hasValue: boolean }>(({ theme, hasValue }) => ({
|
||||
color: hasValue ? 'inherit' : theme.palette.primary.main,
|
||||
'.MuiChip-icon': {
|
||||
transform: 'translateX(50%)',
|
||||
fill: theme.palette.primary.main,
|
||||
height: theme.fontSizes.smallerBody,
|
||||
width: theme.fontSizes.smallerBody,
|
||||
},
|
||||
}));
|
||||
|
||||
interface AddValuesProps {
|
||||
onAddValue: (newValue: string) => void;
|
||||
removeValue: () => void;
|
||||
currentValue?: string;
|
||||
}
|
||||
|
||||
export const AddSingleValueWidget = forwardRef<HTMLDivElement, AddValuesProps>(
|
||||
({ currentValue, onAddValue, removeValue }, ref) => {
|
||||
const [open, setOpen] = useState(false);
|
||||
const positioningRef = useRef<HTMLDivElement>(null);
|
||||
useImperativeHandle(
|
||||
ref,
|
||||
() => positioningRef.current as HTMLDivElement,
|
||||
);
|
||||
|
||||
const handleAdd = (newValue: string, { setError }: OnAddActions) => {
|
||||
if (newValue.length > 100) {
|
||||
setError('Values cannot be longer than 100 characters');
|
||||
return;
|
||||
}
|
||||
|
||||
onAddValue(newValue);
|
||||
setError('');
|
||||
setOpen(false);
|
||||
};
|
||||
|
||||
return (
|
||||
<>
|
||||
<StyledChip
|
||||
hasValue={!!currentValue}
|
||||
ref={positioningRef}
|
||||
label={currentValue || 'Add value'}
|
||||
onClick={() => setOpen(true)}
|
||||
icon={currentValue ? undefined : <Add />}
|
||||
onDelete={currentValue ? removeValue : undefined}
|
||||
/>
|
||||
<AddValuesPopover
|
||||
initialValue={currentValue}
|
||||
onAdd={handleAdd}
|
||||
open={open}
|
||||
anchorEl={positioningRef.current}
|
||||
onClose={() => setOpen(false)}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
},
|
||||
);
|
@ -0,0 +1,127 @@
|
||||
import { Button, Popover, styled, TextField } from '@mui/material';
|
||||
import { ScreenReaderOnly } from 'component/common/ScreenReaderOnly/ScreenReaderOnly';
|
||||
import { type FC, useId, useRef, useState } from 'react';
|
||||
|
||||
const StyledPopover = styled(Popover)(({ theme }) => ({
|
||||
'& .MuiPaper-root': {
|
||||
borderRadius: theme.shape.borderRadiusLarge,
|
||||
border: `1px solid ${theme.palette.divider}`,
|
||||
padding: theme.spacing(2),
|
||||
width: '250px',
|
||||
},
|
||||
}));
|
||||
|
||||
const StyledTextField = styled(TextField)(({ theme }) => ({
|
||||
flexGrow: 1,
|
||||
}));
|
||||
|
||||
const InputRow = styled('div')(({ theme }) => ({
|
||||
display: 'flex',
|
||||
gap: theme.spacing(1),
|
||||
alignItems: 'start',
|
||||
width: '100%',
|
||||
}));
|
||||
|
||||
const ErrorMessage = styled('div')(({ theme }) => ({
|
||||
color: theme.palette.error.main,
|
||||
fontSize: theme.typography.caption.fontSize,
|
||||
marginBottom: theme.spacing(1),
|
||||
}));
|
||||
|
||||
export type OnAddActions = {
|
||||
setError: (error: string) => void;
|
||||
clearInput: () => void;
|
||||
};
|
||||
|
||||
type AddValuesProps = {
|
||||
onAdd: (newValue: string, actions: OnAddActions) => void;
|
||||
initialValue?: string;
|
||||
open: boolean;
|
||||
anchorEl: HTMLElement | null;
|
||||
onClose: () => void;
|
||||
};
|
||||
|
||||
export const AddValuesPopover: FC<AddValuesProps> = ({
|
||||
initialValue,
|
||||
onAdd,
|
||||
anchorEl,
|
||||
open,
|
||||
onClose,
|
||||
}) => {
|
||||
const [inputValue, setInputValue] = useState(initialValue || '');
|
||||
const [error, setError] = useState('');
|
||||
const inputRef = useRef<HTMLInputElement>(null);
|
||||
const inputId = useId();
|
||||
|
||||
return (
|
||||
<StyledPopover
|
||||
open={open}
|
||||
onTransitionEnter={() => {
|
||||
if (inputValue && !initialValue?.trim()) {
|
||||
// if the input value is not empty and the current value is
|
||||
// empty or whitespace
|
||||
setInputValue('');
|
||||
} else if (inputValue) {
|
||||
// select the text in the input field
|
||||
inputRef?.current?.select();
|
||||
}
|
||||
}}
|
||||
disableScrollLock
|
||||
anchorEl={anchorEl}
|
||||
onClose={onClose}
|
||||
anchorOrigin={{
|
||||
vertical: 'bottom',
|
||||
horizontal: 'left',
|
||||
}}
|
||||
transformOrigin={{
|
||||
vertical: 'top',
|
||||
horizontal: 'left',
|
||||
}}
|
||||
>
|
||||
<form
|
||||
onSubmit={(e) => {
|
||||
e.stopPropagation();
|
||||
e.preventDefault();
|
||||
if (!inputValue?.trim()) {
|
||||
setError('Value cannot be empty or whitespace');
|
||||
return;
|
||||
} else {
|
||||
onAdd(inputValue, {
|
||||
setError,
|
||||
clearInput: () => setInputValue(''),
|
||||
});
|
||||
}
|
||||
}}
|
||||
>
|
||||
{error && <ErrorMessage>{error}</ErrorMessage>}
|
||||
<InputRow>
|
||||
<ScreenReaderOnly>
|
||||
<label htmlFor={inputId}>Constraint Value</label>
|
||||
</ScreenReaderOnly>
|
||||
<StyledTextField
|
||||
id={inputId}
|
||||
placeholder='Enter value'
|
||||
value={inputValue}
|
||||
onChange={(e) => {
|
||||
setInputValue(e.target.value);
|
||||
setError('');
|
||||
}}
|
||||
size='small'
|
||||
variant='standard'
|
||||
fullWidth
|
||||
inputRef={inputRef}
|
||||
autoFocus
|
||||
/>
|
||||
<Button
|
||||
variant='text'
|
||||
type='submit'
|
||||
color='primary'
|
||||
disabled={!inputValue?.trim()}
|
||||
>
|
||||
Add
|
||||
</Button>
|
||||
</InputRow>
|
||||
</form>
|
||||
</StyledPopover>
|
||||
);
|
||||
};
|
@ -1,16 +1,11 @@
|
||||
import Add from '@mui/icons-material/Add';
|
||||
import { Button, Popover, styled, TextField } from '@mui/material';
|
||||
import { ScreenReaderOnly } from 'component/common/ScreenReaderOnly/ScreenReaderOnly';
|
||||
import {
|
||||
forwardRef,
|
||||
useId,
|
||||
useImperativeHandle,
|
||||
useRef,
|
||||
useState,
|
||||
} from 'react';
|
||||
import { styled } from '@mui/material';
|
||||
import { forwardRef, useImperativeHandle, useRef, useState } from 'react';
|
||||
import { parseParameterStrings } from 'utils/parseParameter';
|
||||
import { baseChipStyles } from './ValueList';
|
||||
import { AddValuesPopover, type OnAddActions } from './AddValuesPopover';
|
||||
|
||||
// todo: MUI v6 / v7 upgrade: consider changing this to a Chip to align with the rest of the values and the single value selector. There was a fix introduced in v6 that makes you not lose focus on pressing esc: https://mui.com/material-ui/migration/upgrade-to-v6/#chip talk to Thomas for more info.
|
||||
const AddValuesButton = styled('button')(({ theme }) => ({
|
||||
...baseChipStyles(theme),
|
||||
color: theme.palette.primary.main,
|
||||
@ -31,32 +26,6 @@ const AddValuesButton = styled('button')(({ theme }) => ({
|
||||
cursor: 'pointer',
|
||||
}));
|
||||
|
||||
const StyledPopover = styled(Popover)(({ theme }) => ({
|
||||
'& .MuiPaper-root': {
|
||||
borderRadius: theme.shape.borderRadiusLarge,
|
||||
border: `1px solid ${theme.palette.divider}`,
|
||||
padding: theme.spacing(2),
|
||||
width: '250px',
|
||||
},
|
||||
}));
|
||||
|
||||
const StyledTextField = styled(TextField)(({ theme }) => ({
|
||||
flexGrow: 1,
|
||||
}));
|
||||
|
||||
const InputRow = styled('div')(({ theme }) => ({
|
||||
display: 'flex',
|
||||
gap: theme.spacing(1),
|
||||
alignItems: 'start',
|
||||
width: '100%',
|
||||
}));
|
||||
|
||||
const ErrorMessage = styled('div')(({ theme }) => ({
|
||||
color: theme.palette.error.main,
|
||||
fontSize: theme.typography.caption.fontSize,
|
||||
marginBottom: theme.spacing(1),
|
||||
}));
|
||||
|
||||
interface AddValuesProps {
|
||||
onAddValues: (newValues: string[]) => void;
|
||||
}
|
||||
@ -64,17 +33,16 @@ interface AddValuesProps {
|
||||
export const AddValuesWidget = forwardRef<HTMLButtonElement, AddValuesProps>(
|
||||
({ onAddValues }, ref) => {
|
||||
const [open, setOpen] = useState(false);
|
||||
const [inputValues, setInputValues] = useState('');
|
||||
const [error, setError] = useState('');
|
||||
const positioningRef = useRef<HTMLButtonElement>(null);
|
||||
useImperativeHandle(
|
||||
ref,
|
||||
() => positioningRef.current as HTMLButtonElement,
|
||||
);
|
||||
const inputRef = useRef<HTMLInputElement>(null);
|
||||
const inputId = useId();
|
||||
|
||||
const handleAdd = () => {
|
||||
const handleAdd = (
|
||||
inputValues: string,
|
||||
{ setError, clearInput }: OnAddActions,
|
||||
) => {
|
||||
const newValues = parseParameterStrings(inputValues);
|
||||
|
||||
if (newValues.length === 0) {
|
||||
@ -88,9 +56,8 @@ export const AddValuesWidget = forwardRef<HTMLButtonElement, AddValuesProps>(
|
||||
}
|
||||
|
||||
onAddValues(newValues);
|
||||
setInputValues('');
|
||||
clearInput();
|
||||
setError('');
|
||||
inputRef?.current?.focus();
|
||||
};
|
||||
|
||||
return (
|
||||
@ -103,59 +70,13 @@ export const AddValuesWidget = forwardRef<HTMLButtonElement, AddValuesProps>(
|
||||
<Add />
|
||||
<span>Add values</span>
|
||||
</AddValuesButton>
|
||||
<StyledPopover
|
||||
|
||||
<AddValuesPopover
|
||||
onAdd={handleAdd}
|
||||
open={open}
|
||||
disableScrollLock
|
||||
anchorEl={positioningRef.current}
|
||||
onClose={() => setOpen(false)}
|
||||
anchorOrigin={{
|
||||
vertical: 'bottom',
|
||||
horizontal: 'left',
|
||||
}}
|
||||
transformOrigin={{
|
||||
vertical: 'top',
|
||||
horizontal: 'left',
|
||||
}}
|
||||
>
|
||||
<form
|
||||
onSubmit={(e) => {
|
||||
e.stopPropagation();
|
||||
e.preventDefault();
|
||||
handleAdd();
|
||||
}}
|
||||
>
|
||||
{error && <ErrorMessage>{error}</ErrorMessage>}
|
||||
<InputRow>
|
||||
<ScreenReaderOnly>
|
||||
<label htmlFor={inputId}>
|
||||
Constraint Value
|
||||
</label>
|
||||
</ScreenReaderOnly>
|
||||
<StyledTextField
|
||||
id={inputId}
|
||||
placeholder='Enter value'
|
||||
value={inputValues}
|
||||
onChange={(e) => {
|
||||
setInputValues(e.target.value);
|
||||
setError('');
|
||||
}}
|
||||
size='small'
|
||||
variant='standard'
|
||||
fullWidth
|
||||
inputRef={inputRef}
|
||||
autoFocus
|
||||
/>
|
||||
<Button
|
||||
variant='text'
|
||||
type='submit'
|
||||
color='primary'
|
||||
disabled={!inputValues.trim()}
|
||||
>
|
||||
Add
|
||||
</Button>
|
||||
</InputRow>
|
||||
</form>
|
||||
</StyledPopover>
|
||||
/>
|
||||
</>
|
||||
);
|
||||
},
|
||||
|
@ -26,8 +26,10 @@ import { ReactComponent as CaseInsensitiveIcon } from 'assets/icons/case-insensi
|
||||
import { ScreenReaderOnly } from 'component/common/ScreenReaderOnly/ScreenReaderOnly';
|
||||
import { AddValuesWidget } from './AddValuesWidget';
|
||||
import { ResolveInput } from 'component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput';
|
||||
|
||||
import { ReactComponent as EqualsIcon } from 'assets/icons/constraint-equals.svg';
|
||||
import { ReactComponent as NotEqualsIcon } from 'assets/icons/constraint-not-equals.svg';
|
||||
import { AddSingleValueWidget } from './AddSingleValueWidget';
|
||||
|
||||
const Container = styled('article')(({ theme }) => ({
|
||||
'--padding': theme.spacing(2),
|
||||
@ -151,16 +153,16 @@ const StyledCaseSensitiveIcon = styled(CaseSensitiveIcon)(({ theme }) => ({
|
||||
fill: 'currentcolor',
|
||||
}));
|
||||
|
||||
const CaseButton = styled(StyledButton)(({ theme }) => ({
|
||||
display: 'grid',
|
||||
placeItems: 'center',
|
||||
}));
|
||||
|
||||
const OPERATORS_WITH_ADD_VALUES_WIDGET = [
|
||||
'IN_OPERATORS_FREETEXT',
|
||||
'STRING_OPERATORS_FREETEXT',
|
||||
];
|
||||
|
||||
const SINGLE_VALUE_OPERATORS = [
|
||||
'NUM_OPERATORS_SINGLE_VALUE',
|
||||
'SEMVER_OPERATORS_SINGLE_VALUE',
|
||||
];
|
||||
|
||||
type Props = {
|
||||
constraint: IConstraint;
|
||||
localConstraint: IConstraint;
|
||||
@ -212,9 +214,10 @@ export const EditableConstraint: FC<Props> = ({
|
||||
useState(false);
|
||||
const deleteButtonRef = useRef<HTMLButtonElement>(null);
|
||||
const addValuesButtonRef = useRef<HTMLButtonElement>(null);
|
||||
const showSingleValueButton = SINGLE_VALUE_OPERATORS.includes(input);
|
||||
const showAddValuesButton =
|
||||
OPERATORS_WITH_ADD_VALUES_WIDGET.includes(input);
|
||||
const showInputField = !showAddValuesButton;
|
||||
const showInputField = input.includes('LEGAL_VALUES');
|
||||
|
||||
/* We need a special case to handle the currentTime context field. Since
|
||||
this field will be the only one to allow DATE_BEFORE and DATE_AFTER operators
|
||||
@ -359,6 +362,15 @@ export const EditableConstraint: FC<Props> = ({
|
||||
/>
|
||||
) : null}
|
||||
</ValueList>
|
||||
{showSingleValueButton ? (
|
||||
<AddSingleValueWidget
|
||||
onAddValue={(newValue) => {
|
||||
setValue(newValue);
|
||||
}}
|
||||
removeValue={() => setValue('')}
|
||||
currentValue={localConstraint.value}
|
||||
/>
|
||||
) : null}
|
||||
</ConstraintDetails>
|
||||
<ButtonPlaceholder />
|
||||
<HtmlTooltip title='Delete constraint' arrow>
|
||||
|
@ -27,9 +27,9 @@ export const baseChipStyles = (theme: Theme) => ({
|
||||
transition: 'all 0.3s ease',
|
||||
});
|
||||
|
||||
const ValueChipBase = styled(
|
||||
export const ValueChip = styled(
|
||||
forwardRef<HTMLDivElement, ChipProps>((props, ref) => (
|
||||
<Chip size='small' {...props} ref={ref} />
|
||||
<Chip size='small' {...props} ref={ref} deleteIcon={<Clear />} />
|
||||
)),
|
||||
)(({ theme }) => ({
|
||||
...baseChipStyles(theme),
|
||||
@ -44,9 +44,6 @@ const ValueChipBase = styled(
|
||||
'& .MuiChip-deleteIcon': {
|
||||
marginRight: theme.spacing(1),
|
||||
},
|
||||
}));
|
||||
|
||||
const ValueChip = styled(ValueChipBase)(({ theme }) => ({
|
||||
svg: {
|
||||
fill: theme.palette.secondary.dark,
|
||||
borderRadius: '50%',
|
||||
@ -105,7 +102,6 @@ export const ValueList: FC<PropsWithChildren<Props>> = ({
|
||||
whiteSpace: 'normal',
|
||||
},
|
||||
}}
|
||||
deleteIcon={<Clear />}
|
||||
label={value}
|
||||
onDelete={() => {
|
||||
nextFocusTarget(index)?.focus();
|
||||
|
Loading…
Reference in New Issue
Block a user