fix: add isActive to FormAutosuggest state #2517
Conversation
✅ Deploy Preview for paragon-openedx ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
cintnguyen
force-pushed
the
cindy/formAutosuggest-add-isActive-to-state
branch
from
77baba9 to
6852ff3
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2517 +/- ##
==========================================
+ Coverage 91.70% 91.72% +0.01%
==========================================
Files 236 235 -1
Lines 4219 4217 -2
Branches 1021 1020 -1
==========================================
- Hits 3869 3868 -1
+ Misses 346 345 -1
Partials 4 4
☔ View full report in Codecov by Sentry. |
|
@adamstankiewicz just a heads up on this one that the only failing part is codecov (and the overall coverage is actually higher than it was before this PR) |
| e.preventDefault(); | ||
|
|
||
| setState(prevState => ({ | ||
| ...prevState, | ||
| dropDownItems: [], | ||
| errorMessage: !state.displayValue ? errorMessageText : '', |
There was a problem hiding this comment.
[clarification] If removing the errorMessage state here and below, is it correct that this component will no longer be using the errorMessageText prop anymore?
There was a problem hiding this comment.
it seems to still be working on https://deploy-preview-2517--paragon-openedx.netlify.app/components/form/form-autosuggest/
There was a problem hiding this comment.
We're not changing the errorMessage state in the keyDownHandler since the user will still be focused on the component even when they type esc (because this only closes the drop down) #2516. Therefore, we wouldn't want there to be any reported errors until they have left the component.
We still are keeping errorMessageText prop since we use it in handleDocumentClick function above.
cintnguyen
force-pushed
the
cindy/formAutosuggest-add-isActive-to-state
branch
from
6852ff3 to
94d97dc
Compare
|
@cintnguyen LGTM. Looks like there's just a conflict in the |
state.dropDownItems.length > 0 used to determine if an autosuggest component was active or not which was inaccurate. Found an alternative way to specify which component is being interacted with through setting a new bool isActive on the state.
cintnguyen
force-pushed
the
cindy/formAutosuggest-add-isActive-to-state
branch
from
94d97dc to
31df4cc
Compare
|
@adamstankiewicz Merge conflict resolved! |
|
🎉 This PR is included in version 21.1.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 22.0.0-alpha.6 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
state.dropDownItems.length > 0 used to determine if an autosuggest component was active or not which was inaccurate. Found an alternative way to specify which component is being interacted with through setting a new bool isActive on the state.
Resolves : #2510
Deploy Preview
https://deploy-preview-2517--paragon-openedx.netlify.app/components/form/form-autosuggest/
Merge Checklist
wittjeffandadamstankiewiczas reviewers on this PR.Post-merge Checklist