-
Notifications
You must be signed in to change notification settings - Fork 4
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
Infinite FETCH_STTS #2720
Infinite FETCH_STTS #2720
Conversation
…2701-infinite-fetch
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #2720 +/- ##
===========================================
- Coverage 92.99% 92.41% -0.58%
===========================================
Files 219 239 +20
Lines 4482 5366 +884
Branches 385 476 +91
===========================================
+ Hits 4168 4959 +791
- Misses 242 314 +72
- Partials 72 93 +21
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good work!
function STTComboBox({ selectStt, selectedStt, handleBlur, error }) { | ||
const sttList = useSelector((state) => state?.stts?.sttList) | ||
const sttReduction = useSelector((state) => state?.stts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor point but i'd go with something like sttListRequest
for the variable name to indicate what's present in the data structure rather than where in the redux stack it comes from
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…2701-infinite-fetch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great on the a11y front. Noting that I documented a related a11y issue (not introduced by this ticket) in #2735.
More documentation to be added following gov a11y sync tomorrow afternoon with @ttran-hub
@elipe17 is there a way to test this in a deployed environment? also, can you get this deployed to a dev environment when you get a chance? |
…2701-infinite-fetch
a11y LGTM, while potential reading behavior is introduced in #2735. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Pull request closes FETCH_STTS Infinite Request #2701
How to Test
docker-compose.yml
in the backend folder.Request Access
form.python manage.py populate_stts
Request Access
form and verify that you don't get the error modal.Deliverables
More details on how deliverables herein are assessed included here.
Deliverable 1: Accepted Features
Checklist of ACs:
Deliverable 2: Tested Code
CodeCov Report
comment in PR)CodeCov Report
comment in PR)Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
iamjolly
andttran-hub
using Accessibility Insights reveal any errors introduced in this PR?Deliverable 5: Deployed
Deliverable 6: Documented
Deliverable 7: Secure
Deliverable 8: User Research
Research product(s) clearly articulate(s):