[BUGFIX] Fix variable editing panel's custom all value text field to handle empty string by zhuje · Pull Request #64 · perses/shared

zhuje · 2026-02-18T23:17:51Z

Description

Fix the editing variable panel so that the custom all value text field can handle an empty string. Previously, if you tried to delete the current string, the empty string would trigger a hook to repopulate the text field with the previous value.

Screenshots

BEFORE
https://github.com/user-attachments/assets/6eb92fc0-cd22-417c-8fea-e42b34e41fa8

AFTER
https://github.com/user-attachments/assets/c469a264-726a-4035-8bef-2e8583274e3a

Checklist

Pull request has a descriptive title and context useful to a reviewer.
Pull request title follows the [<catalog_entry>] <commit message> naming convention using one of the
following catalog_entry values: FEATURE, ENHANCEMENT, BUGFIX, BREAKINGCHANGE, DOC,IGNORE.
All commits have DCO signoffs.

UI Changes

Changes that impact the UI include screenshots and/or screencasts of the relevant changes.
Code follows the UI guidelines.
E2E tests are stable and unlikely to be flaky.
See e2e docs for more details. Common issues include:
- Is the data inconsistent? You need to mock API requests.
- Does the time change? You need to use consistent time values or mock time utilities.
- Does it have loading states? You need to wait for loading to complete.

…dle empty string Signed-off-by: Jenny Zhu <jenny.a.zhu@gmail.com>

Gladorme · 2026-02-20T21:10:44Z

plugin-system/src/components/Variables/VariableEditorForm/VariableEditorForm.tsx

-                    } else {
-                      field.onChange(event);
-                    }
+                    field.onChange(event.target.value);


We need something on UI side for defining custom all value to undefined then.

A switch?
If enabled: show textfield and handle empty string
If disabled: undefined

Because I am pretty sure some datasources have different behaviors, if the value is empty or undefined

I pushed another commit to handle the case _allAllValue is disabled then the customAllValue should be undefined.

if (!_allowAllValue && values.spec.customAllValue !== undefined) { form.setValue('spec.customAllValue', undefined); }

To me we should handle the case where: _allowAllValue is true and customAllValue can be undefined or empty from the UI too

To me we should handle the case where: _allowAllValue is true and customAllValue can be undefined or empty from the UI too

I took another approach with useState hooks to try to synchronize what the form displays with the value of customAllValue, which was the root of the issue. Now it should handle the case where "_allowAllValue is true and customAllValue can be undefined or empty from the UI too".

Screen.Recording.2026-02-23.at.4.45.12.PM.mov

…dle empty string and undefined Signed-off-by: Jenny Zhu <jenny.a.zhu@gmail.com>

Signed-off-by: Jenny Zhu <jenny.a.zhu@gmail.com>

Gladorme · 2026-02-24T16:22:09Z

plugin-system/src/components/Variables/VariableEditorForm/VariableEditorForm.tsx

+                    onChange={(event) => {
+                      const newValue = event.target.value;
+                      setCustomAllDisplayValue(newValue);
+                      if (newValue === '') {


Now that we can set undefined with reset button, I would remove this check for allowing empty string to be set by UI users

Updated to the Switch approach.

Gladorme · 2026-02-24T16:28:57Z

plugin-system/src/components/Variables/VariableEditorForm/VariableEditorForm.tsx

+  const [customAllDisplayValue, setCustomAllDisplayValue] = useState(_customAllValue ?? '');
+  const [isCustomAllReset, setIsCustomAllReset] = useState(false);
+
+  useEffect(() => {


Arf, not fan of this useEffect, that why I think using a Switch component would be simpler, for enabling or not custom all value.

If switch is enable => show text field and default value is empty string.
If false => textfield hidden and value is undefined

True, Switch is probably the simpler approach. Push another commit to update it to the Switch.

Screen.Recording.2026-02-24.at.4.46.26.PM.mov

Looks better to me, I think useEffect can be removed, no? 🤔

…witch Signed-off-by: Jenny Zhu <jenny.a.zhu@gmail.com>

Gladorme · 2026-02-25T14:53:54Z

plugin-system/src/components/Variables/VariableEditorForm/VariableEditorForm.tsx

+  const [customAllDisplayValue, setCustomAllDisplayValue] = useState(_customAllValue ?? '');
+  const [isCustomAllReset, setIsCustomAllReset] = useState(false);
+
+  useEffect(() => {


Looks better to me, I think useEffect can be removed, no? 🤔

Gladorme · 2026-02-25T14:54:31Z

plugin-system/src/components/Variables/VariableEditorForm/VariableEditorForm.tsx

+                    onChange={(event) => {
+                      if (action === 'read') return;
+                      const isEnabled = event.target.checked;
+                      setUseCustomAllValue(isEnabled);


We can remove the negation here

Suggested change

setUseCustomAllValue(isEnabled);

if (isEnabled) {

form.setValue('spec.customAllValue', '');

} else {

form.setValue('spec.customAllValue', undefined);

}

Gladorme · 2026-02-25T14:55:03Z

plugin-system/src/components/Variables/VariableEditorForm/VariableEditorForm.tsx

+              <Typography variant="caption" sx={{ mt: -0.5 }}>
+                Enable to set a custom value when &quot;All&quot; is selected
+              </Typography>
+              {useCustomAllValue && (


Suggested change

{useCustomAllValue && (

{_customAllValue !== undefined && (

[BUGFIX] fix edit variable panel's custom all value text field to han…

114ca07

…dle empty string Signed-off-by: Jenny Zhu <jenny.a.zhu@gmail.com>

zhuje requested a review from a team as a code owner February 18, 2026 23:17

zhuje changed the title ~~[BUGFIX] fix variable editing panel > custom all value > text field to handle empty string~~ [BUGFIX] Fix variable editing panel's custom all value text field to handle empty string Feb 18, 2026

shahrokni self-requested a review February 19, 2026 09:01

shahrokni approved these changes Feb 19, 2026

View reviewed changes

Gladorme reviewed Feb 20, 2026

View reviewed changes

zhuje added 2 commits February 22, 2026 16:36

[BUGFIX] fix edit variable panel's custom all value text field to han…

fac8185

…dle empty string and undefined Signed-off-by: Jenny Zhu <jenny.a.zhu@gmail.com>

[BUGFIX] fix edit variable panel's custom all value text field

889d936

Signed-off-by: Jenny Zhu <jenny.a.zhu@gmail.com>

Gladorme reviewed Feb 24, 2026

View reviewed changes

[BUGFIX] fix edit variable panel's custom all value text field with S…

c3da52b

…witch Signed-off-by: Jenny Zhu <jenny.a.zhu@gmail.com>

Gladorme reviewed Feb 25, 2026

View reviewed changes

-                      setUseCustomAllValue(isEnabled);
+                      if (isEnabled) {
+                        form.setValue('spec.customAllValue', '');
+                      } else {
+                        form.setValue('spec.customAllValue', undefined);
+                      }

Conversation

zhuje commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Screenshots

Checklist

UI Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuje Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gladorme Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gladorme Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gladorme Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuje Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhuje commented Feb 18, 2026 •

edited

Loading

zhuje Feb 22, 2026 •

edited

Loading

Gladorme Feb 23, 2026 •

edited

Loading

Gladorme Feb 24, 2026 •

edited

Loading

Gladorme Feb 24, 2026 •

edited

Loading

zhuje Feb 24, 2026 •

edited

Loading