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

Bulk Send Batch Progress UI #2305

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Bulk Send Batch Progress UI #2305

wants to merge 3 commits into from

Conversation

codygordon
Copy link
Collaborator

Description

Replaces bulk send batch console logs with a progress UI as well as error handling in the client.

I also went ahead and just completely refactored this component to be functional w/ hooks. I figure it's probably best to write any new components this way if we can as this is the current standard way of creating doing things with React, and in this case it was pretty straightforward to convert it to be fully component-state-based and replace the while loop with a useEffect hook.

If there's an error it will show it as a snackbar alert for 6 seconds if not clicked off, then end sending and reset to be able to show the button again. I still can't figure out how to bubble up actual error messages to the client, and the docs for the old Apollo express package we're using are now offline. That's is something else to note: Apollo packages should be updated soon as we are on a version that is EOL as of this month.

Also noting, we might want to start adding some custom ESLint overrides as we have so many errors/warnings in the codebase they just get ignored. I added our first override in this PR to allow any console output except log, the reasoning being it's sometimes desirable to leave permanent logs in with info, warn, or error, and with this setting it allows those but you can still get warnings if you have a temporary debugging log in there that you actually want to remove, so you don't forget to do that, and any permanent non-error/warning logging can use info.

Anyway, here are a couple screenshots showing how this UI implementation looks:

Screen Shot 2023-10-08 at 4 34 41 PM

Screen Shot 2023-10-08 at 4 33 27 PM

Checklist:

  • I have manually tested my changes on desktop and mobile
  • The test suite passes locally with my changes
  • If my change is a UI change, I have attached a screenshot to the description section of this pull request
  • My change is 300 lines of code or less, or has a documented reason in the description why it’s longer
  • I have made any necessary changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • My PR is labeled [WIP] if it is in progress

package.json Outdated
@@ -232,5 +232,8 @@
"react-tooltip": "^4.2.13",
"recompose": "^0.30.0",
"webpack-cli": "^4.7.2"
},
"browser": {
"crypto": false
Copy link
Collaborator Author

@codygordon codygordon Oct 9, 2023

Choose a reason for hiding this comment

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

Edit: removed this because it was fixed a different way in another PR; confirmed that other fix works for me locally.

crayolakat
crayolakat previously approved these changes Oct 9, 2023
@codygordon codygordon changed the base branch from kathy_bulk_send to main October 27, 2023 20:38
@codygordon codygordon dismissed crayolakat’s stale review October 27, 2023 20:38

The base branch was changed.

@crayolakat crayolakat added the S-ready for stage-main (qa) Status (ADMINS ONLY): PR label for those ready to be added for stage: Approved, tests, etc label Dec 8, 2023
@crayolakat crayolakat mentioned this pull request Dec 8, 2023
@crayolakat crayolakat added S-testing-on-stage Status (ADMINS ONLY): PR label for presence on a staging instance and removed S-ready for stage-main (qa) Status (ADMINS ONLY): PR label for those ready to be added for stage: Approved, tests, etc labels Dec 8, 2023
@crayolakat crayolakat mentioned this pull request Jan 12, 2024
@crayolakat crayolakat mentioned this pull request Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-testing-on-stage Status (ADMINS ONLY): PR label for presence on a staging instance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants