-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
Various fixes in AppListUpdateView #2237
Merged
leonardo-lemos
merged 10 commits into
elementary:main
from
edwood-grant:italo-capasso/various-fixes-AppListUpdateView
Dec 27, 2024
Merged
Various fixes in AppListUpdateView #2237
leonardo-lemos
merged 10 commits into
elementary:main
from
edwood-grant:italo-capasso/various-fixes-AppListUpdateView
Dec 27, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… complete This is done to avoid multiple update check commands that could create duplicated update element in the list.
… updates. This commit also toggles the updates message properly when trying to check new updates. Even when the app closes. This removes all installed and updated apps so iit clears the view whn tyring to update again.
Sionce this only appear when lists are empty, there not need to explicitly remove since we arealready removing list data.
The release notes icon would appear when canelling an update of an application that doew nost have a description (and thus the icon should not appear)
This also fix an issue where installed apps would not show up if your did not open the AppListUpdateView until the update worker ended.
…button instead of action_name. This helps to use the shortuct to update elements and makes the sentiive property of the button more consistent when it starts and ends.
Apartfor being a better palce organizational asking. Thsi also avoids a bug when if you the application while listing updates, it will show a bad update list badge and also would keep the incomplete update list when re-opening the app.
This is needed for when there are no updates. oops.
…n installed_liststore
leonardo-lemos
approved these changes
Dec 27, 2024
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.
Nice work, code looks good and fixes the issues. Thanks!
Generally in the future it would be nice to have at least separate commits and do a rebase merge so that if we have to do a git blame later it would be easier for future maintainers Thank you for resolving these issues! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR make several fixes around the
AppListUpdateView
, and the general Update packages behavior, namely:Fixes Appcenter still reserves space for system updates in eOS 8 #2215 AppCenter still reserves space for system updates
vexpand
property of thelist_box
. When set totrue
it would try to take as much space as possible. It's now set to
false
once the installed panel is visible with any available installed packages.Fixes various issues around not showing properly the updates label.
Disable the "Check for updates" menu button (set sensitive to
false
) when clicking on it, and enable it when the Flatpak backend stops again. This is to stop the user for clicking multiple times (when the button is enabled for brief moments in the check for updates process), which would create duplicate items in the updates view. This fix should also disable it if you are updating any package in the updates list box.Fixes a bug where if you clicked on an application to update that had no description available (and thus no release notes button) and then cancel it immediately, it would show the release notes button.
2024-12-22-03.25.17.131846186.mp4
AppListUpdateView
page and would not open to the main menu until terminated. This also fixes a problem where the view would not show the installed apps if you let the backend check for updates without changing to the updates view until the backend finished.Tests
Here is a video of the whole flow with all the fixes applied:
2024-12-22-05.32.13.712051435.mp4
Comments
I know this is might be a moderate (maybe big?) PR, and I may be doing things wrong here on some places, especially since I don't know the whole picture. I hope it's still good work though to at least keep working on it, or maybe getting closer to something good.
Also, if you'd rather have this split into multiple PRs, or maybe it's not good enough, let me know so I can make it more streamlined or make the necessary changes for everyone.
Thanks a lot,