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

ContentDB GUI: Load package list asynchronously #13551

Merged
merged 7 commits into from Aug 13, 2023

Conversation

grorp
Copy link
Member

@grorp grorp commented Jun 1, 2023

The HTTP request for loading the ContentDB package list is currently done synchronously. This results in a poor user experience and, on Android, sometimes ANRs.

With this PR, the HTTP request is done asynchronously and a "Loading..." dialog is shown in the meanwhile. While I was at it, I also made some small improvements to the ContentDB formspec: I increased the size of some buttons to give translations more space to breathe, and I added a back button to the "No packages could be retrieved" dialog. (Fun fact: The dialog actually already had one, but it was outside the visible area of the formspec in all possible scenarios.)

This is probably best reviewed with whitespace changes hidden.

To do

This PR is a Ready for Review.

How to test

Go to "Content" -> "Browse online content" and enjoy the loading screen and the package list. Verify that an error message is shown if you have no internet connection. Also verify that the loading screen is not shown anymore once the package list has been successfully loaded once.

@grorp
Copy link
Member Author

grorp commented Jun 2, 2023

Here are two screenshots to show the changes I made to the GUI:

before:
screenshot before

after:
screenshot after

Notice that the German translations are less truncated and that there's some space between the scrollbar of the "Bonemeal" package and its delete button. The dialog is still slightly offset to the bottom on desktop, that offset has only been removed on Android.

Copy link
Member

@rubenwardy rubenwardy left a comment

Choose a reason for hiding this comment

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

Looks and works good apart from this

@@ -735,7 +762,30 @@ function store.filter_packages(query)
end
end

local function get_info_formspec(offset, text)
Copy link
Member

@rubenwardy rubenwardy Jun 1, 2023

Choose a reason for hiding this comment

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

Maybe alert would be a better name than info - info could refer to package info. But I don't mind

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Code looks good and works.

@rubenwardy rubenwardy merged commit 526c5f2 into minetest:master Aug 13, 2023
2 checks passed
@grorp grorp deleted the async-fetch-packages branch December 18, 2023 16:35
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

4 participants