-
-
Notifications
You must be signed in to change notification settings - Fork 112
Clone repository: Show progress #1608
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
Conversation
Set input purpose Co-authored-by: Danielle Foré <danielle@elementary.io>
SPDX header Co-authored-by: Danielle Foré <danielle@elementary.io>
# Conflicts fixed: # src/Dialogs/CloneRepositoryDialog.vala # src/MainWindow.vala # src/Services/GitManager.vala
|
@danirabbit As you requested, the cloning dialog now hides during cloning and progress is indicated by a spinner (currently in the choose project button). On cloning success the spinner disappears and a generic toast shows briefly below the choose project button. Is this sufficient or would you like a more informative indication e.g. a progress bar in the sidebar? The remote callbacks are providing more info than is shown at the moment. |
|
I know I have left progress machinery in the dialog at the moment in case this can be re-used elsewhere. |
| private Gtk.Label indexing_label; | ||
| private Gtk.Label progress_label; | ||
| private Gtk.ProgressBar transfer_progress_bar; | ||
| private uint total_objects = 0; |
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.
This class is getting way too large in my opinion. If these widgets are solely to indicate clone progress, they should be marked as so.
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.
I don't think less than 400 lines is particularly large but I'll see whether anything can reasonably be split out. Really need to agree the UI for showing progress before worrying to much about it.
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.
It's not about the lines. It's about the responsibilities.
1 class should have only 1 responsibility.
It's said that our brains can only keep track of 10 things at once. If it's more, it can't handle it. Of course, this depends on the case.
|
I am going to close this as the preferred solution advised by @danirabbit is to just show cloning completion with a toast. It is not therefore necessary to use remote callbacks to get detailed information about the progress of the clone. |
Fixes #1628
Less intrusive solution to indicating cloning in progress (spinner) and finished (toast).