-
Notifications
You must be signed in to change notification settings - Fork 28
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
Proper state/error management for file ingestion #639
Conversation
Deploying opendataeditor with
|
Latest commit: |
7d6ec12
|
Status: | ✅ Deploy successful! |
Preview URL: | https://2c54f3b8.opendataeditor.pages.dev |
Branch Preview URL: | https://622-large-files.opendataeditor.pages.dev |
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.
Thank you @roll for working on these changes. A couple of comments:
Can we change the "Validating..." text for "Checking errors..."?
Apart form that, everything looks ok. One small change, if possible. There seems to a lot of empty space with the introduction of the loading bar on the Upload screen. Can we adjust that a little bit? If this is not possible, merge it. It's not central to the implementation.
Thanks!
Done
I think it's something for @Faithkenny to test amd create a new issue if needed. There is a thing that without this empty space the size of the dialog will be "jumping" when there is loading/messaged/etc. |
OK @roll. Let's implement it as it is for now. |
@romicolman |
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.
This is approved :)
Thanks! |
@romicolman
Please take a look, it fully reworks ingestion data flow and logic that properly covers:
There is still a minor issue with background flickering after validation is done; can't be fixed in the following issues.
PS.
This PR also includes some code cleaning: