Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STCOM-1366 - Selection aggressive 'required' validation. #2363

Merged
merged 6 commits into from
Oct 9, 2024
Merged

Conversation

JohnC-80
Copy link
Contributor

@JohnC-80 JohnC-80 commented Oct 8, 2024

STCOM-1366

React Final Form and redux form validate values using an onblur event. This can cause unwanted validation messages in the UI if the control is shifting focus to its open options list, when the user hasn’t yet had a chance to select a value yet.

Approach

Using refs for the options list containing element and the control itself, check to see if focus is still within the bounds of the component. If it isn't, trigger the provided onBlur if any, and close the menu if necessary.

Before: The control value is validated before any value has been selected, when the menu is opened.
onblur-val-problem

After: Validation only occurs when the user
onblur-val-fix

When the filter input is blurred naturally (tab key) the menu is closed and the onBlur handler is triggered.

Additionally - I noticed issues with keyboard navigation of items in the list and updated dependencies on rendering functions accordingly.

Copy link

github-actions bot commented Oct 8, 2024

Bigtest Unit Test Results

    1 files  ±0      1 suites  ±0   15s ⏱️ -1s
1 525 tests ±0  1 517 ✅ ±0  8 💤 ±0  0 ❌ ±0 
1 527 runs  ±0  1 519 ✅ ±0  8 💤 ±0  0 ❌ ±0 

Results for commit e09c0d8. ± Comparison against base commit bd41e38.

♻️ This comment has been updated with latest results.

@JohnC-80 JohnC-80 changed the title STCOM-1366 - Selection regressive 'required' validation. STCOM-1366 - Selection aggressive 'required' validation. Oct 8, 2024
Copy link

sonarcloud bot commented Oct 8, 2024

@JohnC-80 JohnC-80 requested review from zburke and a team October 9, 2024 14:14
Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

Thank you for the helpful comments explaining what the problem is, how the solution works, and especially the details of why the solution is written as it is with setTimeout. It totally makes sense but is not obvious at first. ❤️

@JohnC-80 JohnC-80 merged commit 64a387c into master Oct 9, 2024
25 checks passed
@JohnC-80 JohnC-80 deleted the STCOM-1366 branch October 9, 2024 19:01
github-actions bot pushed a commit that referenced this pull request Oct 9, 2024
* implement internal onBlur handler for Selection

* add highlightedIndex to renderOptions dependencies. conditionally call onBlur via nullish

* remove commented code

* selection - include closeMenu function in stateReducer
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.

2 participants