Merge lp://staging/~jeremywootten/pantheon-files/transfer-popover-with-pie-progress into lp://staging/~elementary-apps/pantheon-files/trunk

Proposed by Jeremy Wootten
Status: Superseded
Proposed branch: lp://staging/~jeremywootten/pantheon-files/transfer-popover-with-pie-progress
Merge into: lp://staging/~elementary-apps/pantheon-files/trunk
Diff against target: 2642 lines (+1397/-452)
28 files modified
CMakeLists.txt (+3/-2)
data/CMakeLists.txt (+5/-2)
data/pantheon-files.db.service.cmake (+4/-0)
data/pantheon-files.ql.service.cmake (+4/-0)
data/pantheon-files.service.cmake (+1/-0)
libcore/Enums.vala (+9/-0)
libcore/gof-directory-async.vala (+1/-0)
libcore/marlin-file-operations.c (+89/-34)
libcore/marlin-progress-info.c (+190/-2)
libcore/marlin-progress-info.h (+18/-0)
libcore/pantheon-files-core-C.vapi (+7/-0)
libwidgets/CMakeLists.txt (+3/-0)
pantheon-files-daemon/CMakeLists.txt (+48/-17)
pantheon-files-daemon/ProgressHandlerQuicklistInterface.vala (+31/-0)
pantheon-files-daemon/QuicklistHandler.vala (+217/-43)
pantheon-files-daemon/main.vala (+68/-0)
pantheon-files-daemon/marlind-tagging.vala (+0/-58)
pantheon-files-daemon/pantheon-files-daemon.vapi (+28/-0)
src/Application.vala (+57/-9)
src/CMakeLists.txt (+10/-13)
src/ProgressUIHandler.vala (+107/-169)
src/View/Slot.vala (+1/-1)
src/View/Widgets/ProgressIndicator.vala (+105/-0)
src/View/Widgets/ProgressIndicatorWithPopover.vala (+121/-0)
src/View/Widgets/ProgressInfoListRow.vala (+238/-0)
src/View/Widgets/ProgressInfoWidget.vala (+0/-102)
src/View/Widgets/TopMenu.vala (+20/-0)
src/View/Window.vala (+12/-0)
To merge this branch: bzr merge lp://staging/~jeremywootten/pantheon-files/transfer-popover-with-pie-progress
Reviewer Review Type Date Requested Status
Zisu Andrei Pending
Danielle Foré Pending
Review via email: mp+311349@code.staging.launchpad.net

This proposal supersedes a proposal from 2016-03-10.

Description of the change

This branch transfers handling of unity launcher quicklist and progress indicator to pantheon-files daemon.
It also refactors the progress UI handling, replacing the dialog window with a pie (doughnut) shaped progress indicator in the headerbar associated with a popover to show details.

To post a comment you must log in.
Revision history for this message
Danielle Foré (danrabbit) wrote : Posted in a previous version of this proposal

I don't think we need to send a notification when cancelling a transfer. That's something that I just did. There's no delay between me doing it and it happening and it's obvious that it happened because the file is removed from the popover. So it seems unnecessary to send a notification here.

Revision history for this message
Danielle Foré (danrabbit) wrote : Posted in a previous version of this proposal

I don't really like that it warns you about closing the window now. That seems like a regression. Users should be able to close a window without worry and the process just continue to run in the background.

I also get notifications when moving things to the trash from an external disk. That seems unnecessary.

I get a notification on starting a transfer. Again, it seems unnecessary to notify me of things that I'm doing.

Revision history for this message
Jeremy Wootten (jeremywootten) wrote : Posted in a previous version of this proposal

OK, thanks for your comments. I'll work on that.

Revision history for this message
Jeremy Wootten (jeremywootten) wrote : Posted in a previous version of this proposal

The stop process button is now more clearly associated with the progress bar (not a close popover button) and is not symbolic - the red color warning that it has an irreversible effect (is confirmation required?). This follows comments on Google+.

Revision history for this message
Danielle Foré (danrabbit) wrote : Posted in a previous version of this proposal

No, I don't think confirmation is required. It is quite reversible. Just start another transfer :p

Notifications don't seen to have an icon. Probably should use the Files icon so it's clear which app this is coming from.

"File operation completed" is pretty vague. Can we get a little more specific? "5 files copied to Foo" for example?

I seem to always get the HTML mime. I'm guessing this is hardcoded?

Can the gradient on the progressbar go from top to bottom instead of around the doughnut? Looks kinda weird and doesn't really make sense. I'm not sure how the light would have to be to produce that effect :p

I'm thinking maybe we shouldn't group files into a single row. I have two rows that say "Copying foo files to bar". It's hard to remember what exactly each one is.

I'm not sure why you're creating constants for icon names, especially if you're only using it once. Seems like it makes the code less legible.

Any reason you're subclassing gtk.box here instead of using ListBox and ListBoxRow?

Margins on the popover seem oddly huge. Margins on dialogs and popovers should be 12px.

I think it would be helpful to name "ProgressInfoWidget.vala" something more indicative of what it actually is. It seems like that could mean just about anything. Something like "TransferRow" might be more clear about what it is used for and what shape it takes. "Widget" could be anything.

You should use gtk.grid instead of nesting boxes

review: Needs Fixing
Revision history for this message
Jeremy Wootten (jeremywootten) wrote : Posted in a previous version of this proposal

The completion notification is "All file operations have been completed" and only appears when the last of the file operations finishes - there are no notifications for individual notifications for file operations. So your suggestion would only work if there was only one file operation (which, admittedly there usually is). Otherwise you would have to list each operation? And whether or not they were successful or failed? Or notify the ending of each operation individually?

Revision history for this message
Zisu Andrei (matzipan) wrote : Posted in a previous version of this proposal

Tested it. Works nicely, although the pie needs a bit of visual improvements to better match the mockup (https://github.com/elementary/mockups/blob/master/files/transfer.svg). I'll try and do those improvements myself if I have some time.

Diff comments attached to this message.

review: Needs Fixing
2044. By Jeremy Wootten

Merge trunk to r2444 and resolve conflicts

2045. By Jeremy Wootten

Move Progress widgets to src/View/Widgets

2046. By Jeremy Wootten

Define MarlinFileOperationType in marlin-progress-info.h not Enums.vala to avoid conflicts with testing branches

2047. By Jeremy Wootten

Merge trunk to r2470

Unmerged revisions

2047. By Jeremy Wootten

Merge trunk to r2470

2046. By Jeremy Wootten

Define MarlinFileOperationType in marlin-progress-info.h not Enums.vala to avoid conflicts with testing branches

2045. By Jeremy Wootten

Move Progress widgets to src/View/Widgets

2044. By Jeremy Wootten

Merge trunk to r2444 and resolve conflicts

2043. By Jeremy Wootten

Merge trunk to r2382 and resolve conflicts

2042. By Jeremy Wootten

Merge trunk to r2229 and resolve conflicts. Make prefix for APP_ID, .desktop files, dbus service files, dbus service addresses, settings schemas consistently org.pantheon.files

2041. By Jeremy Wootten

Change total_files to total_file_count

2040. By Jeremy Wootten

Change remaining_files to remaining_file_count

2039. By Jeremy Wootten

Rename info->no_files

2038. By Jeremy Wootten

Initialise info->current_filename; return empty string if null

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: