Skip to content

fix: click empty http node value may cause blur#35051

Merged
crazywoola merged 1 commit intomainfrom
fix/http-value-unblur
Apr 13, 2026
Merged

fix: click empty http node value may cause blur#35051
crazywoola merged 1 commit intomainfrom
fix/http-value-unblur

Conversation

@iamjoel
Copy link
Copy Markdown
Contributor

@iamjoel iamjoel commented Apr 13, 2026

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran make lint && make type-check (backend) and cd web && pnpm exec vp staged (frontend) to appease the lint gods

@iamjoel iamjoel requested a review from zxhlyh as a code owner April 13, 2026 09:54
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Apr 13, 2026
@github-actions github-actions Bot added the web This relates to changes on the web. label Apr 13, 2026
@iamjoel
Copy link
Copy Markdown
Contributor Author

iamjoel commented Apr 13, 2026

@copilot

@iamjoel
Copy link
Copy Markdown
Contributor Author

iamjoel commented Apr 13, 2026

@copilot review this pr

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 13, 2026

@copilot review this pr

Here's my review of the PR:

Problem being fixed:
When clicking the value container of the last (empty) key-value row in an HTTP node, onAdd() was always fired — even when the value was empty — causing the newly focused input to blur immediately.

Changes in item.tsx:

  1. hasValuePayload — derives whether the current row has a value (payload.value for text, payload.file length for file type). Clean and correct.

  2. handleValueContainerClick — replaces the inline onClick={() => isLastItem && onAdd()} with a guard that only calls onAdd() when the row already has a value. This is the core fix: clicking an empty row no longer triggers a blur.

  3. handleChange — adds shouldAddNextItem logic so that when the user types the first value into the last row, onAdd() is still called. This preserves the auto-append behavior when filling in values.

    • The shouldAddNextItem check correctly captures the pre-change state (!payload.value) before calling onChange, avoiding stale closure issues.
  4. useCallback deps[isLastItem, onAdd, onChange, payload] is correct for handleChange; [hasValuePayload, isLastItem, onAdd] is correct for handleValueContainerClick.

Test in integration.spec.tsx:
The new test validates both sides of the fix: clicking an empty value field does not call onAdd, but entering a value does. Good coverage of the regression.

Minor observation:
handleValueContainerClick will still call onAdd() on every click of a row that already has a value (same as before the fix). If the parent doesn't deduplicate, repeated clicks could append multiple empty rows. This was pre-existing behaviour and is out of scope for this fix, but worth noting.

Overall the fix is clean, minimal, and correctly targeted. ✅

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.87%. Comparing base (ae89865) to head (a91fd22).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #35051   +/-   ##
=======================================
  Coverage   84.87%   84.87%           
=======================================
  Files        4360     4360           
  Lines      195784   195795   +11     
  Branches    37230    37237    +7     
=======================================
+ Hits       166165   166176   +11     
  Misses      26569    26569           
  Partials     3050     3050           
Flag Coverage Δ
web 85.76% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Apr 13, 2026
@crazywoola crazywoola added this pull request to the merge queue Apr 13, 2026
Merged via the queue into main with commit 28fce0a Apr 13, 2026
34 of 35 checks passed
@crazywoola crazywoola deleted the fix/http-value-unblur branch April 13, 2026 14:57
HanqingZ pushed a commit to HanqingZ/dify that referenced this pull request Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files. web This relates to changes on the web.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants