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

versions: Improve the loading indicator #10740

Merged
merged 1 commit into from
Mar 5, 2025

Conversation

eth3lbert
Copy link
Contributor

An excerpt from https://rust-lang.zulipchat.com/#narrow/channel/318791-t-crates-io/topic/swc/near/503007257

I noticed that while loading the versions list there is no loading indicator and it looks like the crate simply has no versions. might be worth implementing some kind of loading screen for that state.

This shows a loading indicator for the initial load, and a loading button when loading more :D

Screen.Recording.2025-03-03.at.21.34.36.mov

@eth3lbert eth3lbert added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-frontend 🐹 labels Mar 3, 2025
@eth3lbert eth3lbert requested a review from Turbo87 March 3, 2025 13:38
@eth3lbert
Copy link
Contributor Author

I need to fix those flaky CI issues that aren't related to this PR. It's quite annoying :/

{{#each this.sortedVersions as |version|}}
<li>
<VersionList::Row @version={{version}} local-class="row" data-test-version={{version.num}} />
</li>
{{/each}}
</ul>
{{#if this.next_page}}
{{#if this.loadMoreTask.isRunning}}
Copy link
Member

Choose a reason for hiding this comment

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

should we also hide the .page-description element above while this.loadMoreTask.isRunning && this.sortedVersions.count === 0?

Copy link
Contributor Author

@eth3lbert eth3lbert Mar 4, 2025

Choose a reason for hiding this comment

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

So, this is regarding the initial load (like the 'is-empty' check mentioned above)? I presume this could be achieved via

style="{{unless this.sortedVersions 'visibility:hidden'}}"

(which could also be implemented using a CSS class and style).

I don't really have a preference on this, but I think the reasoning behind it is to make it more noticeable that it's still loading and is an initial load. So, I'm okay with this.

I'm also wondering if we should prevent the page loading progress (the yellow one at the top) from completing until the first versions have finished loading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should we also hide the .page-description element above while this.loadMoreTask.isRunning && this.sortedVersions.count === 0?

done!

This shows a loading indicator for the initial load, and a loading
button when loading more.
@eth3lbert eth3lbert force-pushed the app-versions-loading branch from d920fba to 4dbc9ba Compare March 4, 2025 09:32
Copy link
Member

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

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

looks good, thanks 👍

@Turbo87 Turbo87 merged commit e04d9e6 into rust-lang:main Mar 5, 2025
11 checks passed
@eth3lbert eth3lbert deleted the app-versions-loading branch March 5, 2025 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend 🐹 C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants