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

loading and error enhancements in studio #9225

Merged
merged 9 commits into from Nov 13, 2023

Conversation

aditya-mitra
Copy link
Collaborator

@aditya-mitra aditya-mitra commented Nov 7, 2023

Summary

Show a loading indicator beside the entity whose components are being loaded.
Also fix the error tooltip which was not properly showing the detailed errors.

Note: I replaced hasComponent(entity, myComponent) set/removeComponent with just set/removeComponent as both check for entity before setting/removing

References

closes part of #9201

QA Steps

3fe46b43-eda4-4db8-80cb-cf551d9dbb24.mp4

Screenshot 2023-11-10 at 7 16 02 PM

@aditya-mitra aditya-mitra marked this pull request as ready for review November 10, 2023 13:52
@AidanCaruso AidanCaruso self-requested a review November 10, 2023 18:14
Copy link
Member

@AidanCaruso AidanCaruso left a comment

Choose a reason for hiding this comment

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

Looks really good, definitely improves the loading/error tips. However, I feel the need to point out that this is hampered a bit by the model component's error handling logic generally being wrong. Nonexistant models returning 404s seemingly aren't always caught if they are of a supported asset type, and invalid URLs get flagged as invalid asset types. This can be fixed in another PR down the line.

@aditya-mitra aditya-mitra added this pull request to the merge queue Nov 13, 2023
Merged via the queue into dev with commit 3a04381 Nov 13, 2023
13 checks passed
@aditya-mitra aditya-mitra deleted the feat/studio-loading-enhancements branch November 13, 2023 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants