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 indicator for line and x-range layers #1017

Merged
merged 3 commits into from
Dec 20, 2023

Conversation

AaronPlave
Copy link
Contributor

@AaronPlave AaronPlave commented Nov 27, 2023

Display a loading indicator on a row when the row has at least one layer and resources are loading. Abort stale resource requests and avoid unnecessary refetching of resources.

@AaronPlave AaronPlave requested a review from a team as a code owner November 27, 2023 17:05
@AaronPlave AaronPlave self-assigned this Nov 27, 2023
@AaronPlave AaronPlave added the feature New feature or request label Nov 27, 2023
@AaronPlave AaronPlave force-pushed the feat/resource-layer-loading-indicator branch from 45fbd84 to 61ff409 Compare December 6, 2023 16:31
Copy link
Collaborator

@duranb duranb left a comment

Choose a reason for hiding this comment

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

🥇

@AaronPlave
Copy link
Contributor Author

Hold up – not necessarily an issue with this PR but seeing that we're fetching resources and external resources every time the simulation progress updates which causes a flash of the resource loading message. Can see this on Bananation, not sure about other models. Will investigate.

@AaronPlave AaronPlave requested a review from duranb December 11, 2023 19:25
@AaronPlave AaronPlave force-pushed the feat/resource-layer-loading-indicator branch from 799feef to f3087ae Compare December 11, 2023 19:25
@AaronPlave
Copy link
Contributor Author

Made some fixes to how we load resources and spans in the plan page to avoid duplicative/unnecessary fetches. Also now aborting stale external resource, resource, and span requests. Ready for re-review.

Copy link
Contributor

@jeffpamer jeffpamer left a comment

Choose a reason for hiding this comment

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

Tested it out a bit locally and changes look good!

Copy link
Collaborator

@duranb duranb left a comment

Choose a reason for hiding this comment

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

Probably also not related to this PR, but just exposed by it, I'm seeing the loading text show up if I drag a directive but haven't run a simulation yet. The text does eventually go away once everything is downloaded again. I'll leave it up to your discretion if it's something that needs to be addressed. Other than that, I think it's good to go!

@AaronPlave
Copy link
Contributor Author

Probably also not related to this PR, but just exposed by it, I'm seeing the loading text show up if I drag a directive but haven't run a simulation yet. The text does eventually go away once everything is downloaded again. I'll leave it up to your discretion if it's something that needs to be addressed. Other than that, I think it's good to go!

Can't reproduce this yet, can you show me once you're back?

…tor. Delay loading indicator by 1 second to prevent flash of content for quick loading resources.
@AaronPlave AaronPlave force-pushed the feat/resource-layer-loading-indicator branch from f3087ae to bb3b1a5 Compare December 20, 2023 00:08
@AaronPlave AaronPlave merged commit 32d3a80 into develop Dec 20, 2023
4 checks passed
@AaronPlave AaronPlave deleted the feat/resource-layer-loading-indicator branch December 20, 2023 00:18
JosephVolosin pushed a commit that referenced this pull request Aug 20, 2024
* Display a loading indicator when a line or x-range layer is awaiting resource download
* Consider external resource loading when presenting row loading indicator. Delay loading indicator by 1 second to prevent flash of content for quick loading resources.
* Fix redundant resource and span fetches during simulation progress updates and abort stale requests.
JosephVolosin pushed a commit that referenced this pull request Oct 21, 2024
* Display a loading indicator when a line or x-range layer is awaiting resource download
* Consider external resource loading when presenting row loading indicator. Delay loading indicator by 1 second to prevent flash of content for quick loading resources.
* Fix redundant resource and span fetches during simulation progress updates and abort stale requests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants