Skip to content

SCIX-846 Fix List Components in Feedback Forms#871

Merged
thostetler merged 1 commit into
adsabs:masterfrom
thostetler:scix-846
May 29, 2026
Merged

SCIX-846 Fix List Components in Feedback Forms#871
thostetler merged 1 commit into
adsabs:masterfrom
thostetler:scix-846

Conversation

@thostetler
Copy link
Copy Markdown
Member

@thostetler thostetler commented May 27, 2026

Feedback forms that use a staging input pattern (type a value, click + to commit) silently
discard the staged value when users blur away without clicking the button. Single-entry
users who type one value and move directly to Preview find the form stuck with the button
disabled and no explanation.

  • Add container-level onBlur to the new-bibcode group in AssociatedTable, the new-URL row in
    UrlsTable, and the new-reference row in MissingReferenceTable
  • Auto-commit the staged value when focus leaves the container, if the value is valid
  • Guard with e.currentTarget.contains(e.relatedTarget) to skip auto-commit when focus moves
    between siblings within the same row, preventing double-commits when the user clicks the
    explicit Add button
    Kooha-2026-05-27-14-16-52.webm

Copilot AI review requested due to automatic review settings May 27, 2026 04:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Risk summary: medium; the blur auto-commit behavior is targeted, but the current implementation can pull focus back into the staging rows after users intentionally move away.

This PR addresses staged feedback-form list inputs that previously lost typed-but-uncommitted values when users blurred away without clicking the add button.

Changes:

  • Adds container-level blur handlers to auto-commit valid staged bibcode, URL, and missing-reference entries.
  • Uses contains(relatedTarget) guards to avoid committing while focus moves within the same add row/group.
  • Adds tests covering blur auto-commit and invalid/in-row focus movement cases.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/components/FeedbackForms/AssociatedArticles/AssociatedArticlesForm.tsx Adds blur auto-commit for associated bibcode staging input.
src/components/FeedbackForms/AssociatedArticles/AssociatedArticlesForm.test.tsx Adds tests for associated bibcode blur commit behavior.
src/components/FeedbackForms/MissingRecord/UrlsField.tsx Adds blur auto-commit for the staged URL row.
src/components/FeedbackForms/MissingRecord/UrlsField.test.tsx Adds tests for valid, invalid, and in-row URL blur behavior.
src/components/FeedbackForms/MissingReferences/MissingReferenceTable.tsx Adds blur auto-commit for the staged missing-reference row.
src/components/FeedbackForms/MissingReferences/MissingReferenceTable.test.tsx Adds tests for missing-reference blur commit behavior.

Comment thread src/components/FeedbackForms/AssociatedArticles/AssociatedArticlesForm.tsx Outdated
Comment thread src/components/FeedbackForms/MissingRecord/UrlsField.tsx Outdated
Comment on lines +111 to +112
if (newReference.citing.length > 0 && newReference.cited.length > 0) {
handleAddReference();
@codecov
Copy link
Copy Markdown

codecov Bot commented May 27, 2026

Codecov Report

❌ Patch coverage is 92.59259% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.2%. Comparing base (58aca9c) to head (fc60985).

Files with missing lines Patch % Lines
...orms/AssociatedArticles/AssociatedArticlesForm.tsx 63.7% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #871     +/-   ##
========================================
- Coverage    62.6%   61.2%   -1.4%     
========================================
  Files         324     348     +24     
  Lines       38203   41371   +3168     
  Branches     1733    1818     +85     
========================================
+ Hits        23905   25281   +1376     
- Misses      14257   16046   +1789     
- Partials       41      44      +3     
Files with missing lines Coverage Δ
...ts/FeedbackForms/MissingRecord/ReferencesField.tsx 68.1% <100.0%> (ø)
...mponents/FeedbackForms/MissingRecord/UrlsField.tsx 68.6% <100.0%> (ø)
...kForms/MissingReferences/MissingReferenceTable.tsx 68.4% <100.0%> (ø)
...orms/AssociatedArticles/AssociatedArticlesForm.tsx 67.6% <63.7%> (ø)

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@thostetler thostetler force-pushed the scix-846 branch 5 times, most recently from 34cd25d to b5b7626 Compare May 27, 2026 17:24
… forms

Three feedback form components (AssociatedTable, UrlsTable,
MissingReferenceTable) used a staging-input pattern where typed values
were silently discarded if the user blurred without clicking the explicit
Add button. Single-entry users who typed a value and moved directly to
Preview were left with an empty form and no error message.

Fix: add container-level onBlur handlers with a
`e.currentTarget.contains(e.relatedTarget)` guard to auto-commit staged
values when focus leaves the container, while ignoring focus moves
between siblings within the same row. Blur-triggered commits skip
focus-restore; explicit Add button commits retain it.

UrlsField also guards against premature auto-commit when the portaled
react-select menu is open (onMenuOpen/onMenuClose flag).

Adds unit tests for all three components covering the happy path,
partial-fill no-op, and sibling-focus guard.
Copy link
Copy Markdown
Member

@shinyichen shinyichen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@thostetler thostetler merged commit 7dce5ed into adsabs:master May 29, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants