From cb4beb71acfc7b690291885aaeab9f51c717ed13 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 4 Jul 2025 14:57:52 +0200 Subject: [PATCH] Chore(1 3895)/small input tweaks (#10316) Doesn't clear the value from the constraint input value popover if you close it and then re-open. In other words, if you accidentally click out, you don't lose your progress. Instead, the popover will open again, with the value you had when you closed it highlighted (so that it's easy to type over if you want to): image The reason I'm changing this now is because I noticed that the error wasn't cleared correctly when the popover was closed. If we do it this way instead, then that makes sense, because you can still see the value. This is also how the single-value popover has worked forever. From some quick testing, the single value popover still works as expected: image As a side note: I'm adding a comment to anyone coming after as to why focus handling on escape doesn't work correctly on the single value button. I was about to go down a rabbit hole on that before I read my own comment on the previous PR. So I thought I'd put that here too. --- .../EditableConstraint/AddSingleValueWidget.tsx | 7 +++++++ .../EditableConstraint/AddValuesPopover.tsx | 9 +-------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/AddSingleValueWidget.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/AddSingleValueWidget.tsx index 91ff7b408c..200132fa78 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/AddSingleValueWidget.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/AddSingleValueWidget.tsx @@ -5,6 +5,13 @@ import { ValueChip } from './ValueList.tsx'; import { AddValuesPopover, type OnAddActions } from './AddValuesPopover.tsx'; import type { ConstraintValidatorOutput } from './ConstraintValidatorOutput.ts'; +// No, escape handling doesn't work correctly with chips and popovers in MUI v5. +// This is an intentional trade-off for now because the chip makes it easy to +// use delete/backspace to clear the value and otherwise works pretty well (cf +// https://github.com/Unleash/unleash/pull/9859). +// +// In MUI v6 and onwards this will "just work" +// (https://mui.com/material-ui/migration/upgrade-to-v6/#chip) const StyledChip = styled(ValueChip, { shouldForwardProp: (prop) => prop !== 'hasValue', })<{ hasValue: boolean }>(({ theme, hasValue }) => ({ diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/AddValuesPopover.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/AddValuesPopover.tsx index 6da67aa321..398470b404 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/AddValuesPopover.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/AddValuesPopover.tsx @@ -80,14 +80,7 @@ export const AddValuesPopover: FC = ({ { - 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(); - } + inputRef?.current?.select(); }} disableScrollLock anchorEl={anchorEl}