fix: experiment form UX fixes - #8251
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe changes control typed input in Estimated code review effort: 2 (Simple) | ~10 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
front-end
Docker builds report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 99e25f14-b941-426a-95b3-98a8837b72aa
📒 Files selected for processing (4)
frontend/web/components/PageTitle.tsxfrontend/web/components/experiments/EventNameSelect/EventNameSelect.tsxfrontend/web/components/experiments/results/ExperimentSummaryScorecard.tsxfrontend/web/components/experiments/steps/SetupStep.tsx
| const [inputValue, setInputValue] = useState('') | ||
|
|
||
| // Keep the typed text on blur (react-select discards it by default) so | ||
| // clicking outside commits the value instead of clearing it. | ||
| const handleInputChange = (val: string, meta: InputActionMeta) => { | ||
| if (meta.action === 'input-change') setInputValue(val) | ||
| if (meta.action === 'set-value') setInputValue('') | ||
| } | ||
| const handleBlur = () => { | ||
| if (!inputValue) return | ||
| onChange(inputValue) | ||
| setInputValue('') | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear the draft when the user clears the selection.
react-select 5.9.0 sends the clear-indicator action through onChange with action: 'clear'. Option selection uses the separate onInputChange set-value path handled here. (raw.githubusercontent.com)
If the user types a draft, clicks the clear control, and then blurs the field, inputValue remains populated. handleBlur can then call onChange(inputValue) and restore the cleared event.
Handle actionMeta.action === 'clear' in the select onChange callback. Add a regression test for draft input followed by clear and blur.
Suggested fix
- onChange={(option: EventOption | null) => onChange(option?.value ?? '')}
+ onChange={(option: EventOption | null, actionMeta) => {
+ if (actionMeta.action === 'clear') setInputValue('')
+ onChange(option?.value ?? '')
+ }}Verification script
#!/bin/bash
set -euo pipefail
select_source="$(
fd -t f 'Select.tsx' . 2>/dev/null |
while IFS= read -r file; do
if rg -q "action: 'clear'|action: 'set-value'" "$file"; then
printf '%s\n' "$file"
break
fi
done
)"
if [[ -z "$select_source" ]]; then
printf '%s\n' 'Could not locate the react-select source.' >&2
exit 2
fi
rg -n -C 3 "action: 'clear'|action: 'set-value'" "$select_source"
rg -n -C 6 'handleInputChange|handleBlur|onChange=' \
frontend/web/components/experiments/EventNameSelect/EventNameSelect.tsx
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19158 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
🗂️ Previous results✅ oss · depot-ubuntu-latest-arm-16 — run #19158 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #19158 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
1 participant
docs/if required so people know about the feature.Changes
Small experiment UX fixes:
ms-lg-2on thePageTitleCTA that indented the button when it wraps below the description (e.g. Segments page).How did you test this code?
Manually.