-
Notifications
You must be signed in to change notification settings - Fork 437
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
Fix: hide Monitors without a patient checkbox filter is not toggling #6992
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
✅ Deploy Preview for care-egov-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
why is a state required? query params itself holds the state and it worked before that way.
If I don't use a state then the checkbox doesn't get checked or unchecked quickly, there's some noticeable delay when you do so. |
why is the noticeable delay being caused? |
Sorry my bad, the delay was seemed to have caused because of my computer slowing down. |
But there's still a noticeable delay when you click the checkbox and the monitors (without patients) appearing and reappearing due to the api call. |
} else { | ||
removeFilter(name); | ||
} | ||
updateQuery({ [name]: value }); |
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.
are you sure this is right? we don't want to filter by bed_is_occupied==False
when unchecked, rather we don't want to perform the filter itself based on bed_is_occupied
altogether when unchecked right?
Yup that sounds good! We can show the CARE loading icon when the useQuery's |
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.
I've pushed the fix (and also shows the loading component when refetching) as this had to be fixed by today evening. cc: @nihal467
The issue was the query param variable is of type string, but the checkbox expected value of type boolean. Hence the issue.
@rithviknishad Thanks for pushing a fix. I'm so sorry for not seeing your comment earlier and fixing it. |
LGTM |
@GokulramGHV We truly appreciate your efforts. Thank you for taking the time to contribute; this is a very valuable contribution to us 🥇. We always welcome your contribution 🙂, so feel free to contribute to anything anytime, and never lose that spirit of innovation 🙌. |
Proposed Changes
@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers
Merge Checklist