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

Add spinners and disable forms while data is being loaded - alternate #3050

Merged
merged 11 commits into from
Nov 3, 2023

Conversation

alexcottner
Copy link
Contributor

Alternate (hopefully slightly cleaner) PR compared to 3049.

The goal of this PR is to prevent users from interacting with forms while we're waiting on server responses. I think the easiest way to do that is to:

  1. Disable the forms while we're waiting for responses after user submissions/onChange events that trigger API calls
  2. Provide a visual cue to users that data is being loaded

This should resolve the following open issues:

  1. Display a busy state whenever the server info is fetched
  2. Prevent double submission of forms
  3. Simple review should disable the approve/reject buttons after they're clicked
  4. Loading indicator not shown when loading history
simplescreenrecorder-2023-11-01_11.41.55.mp4

Copy link
Contributor

@grahamalama grahamalama left a comment

Choose a reason for hiding this comment

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

Nice! I'll throw an approval here for now, though you said you're still going to add some tests to cover, #1360, #2206, and #1596 right?

Comment on lines +42 to +44
beforeEach(() => {});

afterEach(() => {});
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope! Added them out of habit and forgot to remove them because prettier made them too small. I noticed them in the next branch and will be removing them then.

test/components/BaseForm_test.js Outdated Show resolved Hide resolved
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.

None yet

2 participants