GNOME Bugzilla – Bug 761857
Downloads popover - visual and layout issues
Last modified: 2016-02-27 18:38:25 UTC
Created attachment 320867 [details] screenshot of current state and mockup proposal The new downloads popover is great, but it still has quite a lot of rough edges. Here's a list I came up with when testing today: * Open button for completed downloads is ambiguous - hard to know what the icon means. * No way to open a downloaded file directly - you have to open the downloads folder and then open it. * Downloads label isn't horizontally centered within the popover. * Dimensions of the popover look wrong - it is too squat. * Bold text for the file name is too heavy. * White list background is ugly. * File icons are too dominant * Empty space where progress bar would be for completed downloads leaves an odd layout. * Not enough padding between list rows - makes it hard to ready. * List buttons aren't horizontally aligned with the clear button. I've attached a mockup to provide some approaches to fixing these issues. Specific changes it includes: * Use 16x16 symbolic icons to indicate the file type, and show them at the beginning of the file name. * Don't use bold text for the file name. * Increase padding between list rows. * Use a consistent background color, and remove the white background for the list. * Replace the icon in the open button for a label. * When a download finishes, change the layout of the list row to avoid the gap where the progress bar would have been. * Make the open button open the file directly, and add a separate Downloads Folder button at the bottom. * Change "Clear" to "Clear All" (this is more accurate description of the action), and move to the bottom. * Increase the height of the popover.
Created attachment 321723 [details] [review] downloads-popover: Fix visual and layout issues The complete list of issues and solutions can be found in the bug report.
Created attachment 321724 [details] Screenshot of it with the patch applied This is how it currently looks with the patch applied. I'm not sure I got the dimensions right. A problem I encountered and I was unable to fix is the popover getting resized to fit the "Open" button when the download is finished.
Review of attachment 321723 [details] [review]: Great job, it's not often an implementation matches the mockup so precisely. We will have to figure out what to do about popover resizing. In the meantime, you get to learn about freeze breaks. :) Please mail release-team@gnome.org (CC me) and ask for a UI freeze break for this. Justification: this popover is brand new in 3.20; it's not good to introduce it in one release and redesign it in the next, so we should land this now. ::: lib/widgets/ephy-download-widget.c @@ +288,3 @@ + return; + + ephy_download_do_download_action (widget->download, FWIW, an existing bug and not something to address in this patch: there is a bug where sometimes this does nothing, I think if the file is not on the list of "safe" types. The list of safe types makes sense to prevent certain risky file types from being opened automatically, but it shouldn't break our UI when users click open. @@ +446,3 @@ + + if (ephy_download_succeeded (widget->download)) + { In this file we use K&R braces: if { } else { } But we omit the braces when there is only one statement, so just get rid of the braces here.
Created attachment 321732 [details] [review] downloads-popover: Fix visual and layout issues The complete list of issues and solutions can be found in the bug report.
(In reply to Michael Catanzaro from comment #3) > We will have to figure out what to do about popover resizing. It seems hard to fix; let's not block on this, unless Carlos has a suggestion.
Created attachment 321736 [details] [review] downloads-popover: Fix GtkListBoxRow not being destroyed This should fix the GtkListBoxRow that was not removed with the download widget.
Review of attachment 321732 [details] [review]: It looks good. Here's a suggestion for the resize problem: try putting the cancel and open buttons into a GtkStack, and use gtk_stack_set_hhomogeneous() -- that should ensure that the same width is allocated for both children. I think it will work; the risk is that it expands the cancel button too much, in which case a workaround would be for the children of the stack to be the open button and a GtkBox, and pack the cancel button into the GtkBox with fill=FALSE. Worth a try.
Review of attachment 321736 [details] [review]: ::: lib/widgets/ephy-downloads-popover.c @@ +107,3 @@ if (ephy_download_widget_get_download (EPHY_DOWNLOAD_WIDGET (widget)) == download) { gtk_widget_destroy (widget); + gtk_widget_destroy (l->data); I think you only need to destroy the GtkListBoxRow, that should destroy all its child widgets, including the EphyDownloadWidget.
Review of attachment 321732 [details] [review]: This is not just changing the design, but also the behavior. I don't understand why we have changed the open in folder button, that was consistent with the cancel button, with a huge rectangular open button. The downloads folder button can't replace the open in folder button of a download. The advantage of the browse to action is that it not only opens nautilus in the downloads folder but also selects the downloaded file. This is very convenient and avoids having to search for the download in the folder. Open in container action allows you to do whatever you want with the download, moving to another folder, or just opening it, much better than our open action. So, I think open in containing folder should be the most important action for a download, more than open, that is still possible by double clicking the download (I admit is a hidden, though). ::: lib/widgets/ephy-download-widget.c @@ -282,3 @@ - } else { - ephy_download_do_download_action (widget->download, - EPHY_DOWNLOAD_ACTION_BROWSE_TO); Why are we removing this? @@ +285,3 @@ +widget_open_button_clicked_cb (EphyDownloadWidget *widget) +{ + if (!ephy_download_succeeded (widget->download)) Are we showing the button for failed downloads? and is it enabled? @@ -415,3 @@ - if (ephy_download_succeeded (widget->download)) - action_icon_name = "folder-open-symbolic"; Why are we using a different button for succeeded downloads? ::: lib/widgets/ephy-downloads-popover.c @@ +118,2 @@ static void +downloads_button_clicked_cb (EphyDownloadsPopover *popover) downloads_folder_button_clicked_cb
(In reply to Carlos Garcia Campos from comment #9) > So, I think open in containing folder should be the most > important action for a download, more than open, that is still possible by > double clicking the download (I admit is a hidden, though). Allan, what do you think? I guess we would not need the Downloads Folder button then. > ::: lib/widgets/ephy-download-widget.c > @@ -282,3 @@ > - } else { > - ephy_download_do_download_action (widget->download, > - EPHY_DOWNLOAD_ACTION_BROWSE_TO); By the way, when removing something like this in the future, 'git grep' for other uses and clean them up, so we don't wind up with dead code.
(In reply to Michael Catanzaro from comment #10) > (In reply to Carlos Garcia Campos from comment #9) > > So, I think open in containing folder should be the most > > important action for a download, more than open, that is still possible by > > double clicking the download (I admit is a hidden, though). > > Allan, what do you think? I guess we would not need the Downloads Folder > button then. I'm fine with the downloads button, it's just different opening the downloads folder from opening a particular download in containing folder. What I don't understand is the huge open button for finished downloads. > > ::: lib/widgets/ephy-download-widget.c > > @@ -282,3 @@ > > - } else { > > - ephy_download_do_download_action (widget->download, > > - EPHY_DOWNLOAD_ACTION_BROWSE_TO); > > By the way, when removing something like this in the future, 'git grep' for > other uses and clean them up, so we don't wind up with dead code.
(In reply to Iulian Radu from comment #2) > Created attachment 321724 [details] > Screenshot of it with the patch applied > > This is how it currently looks with the patch applied. I'm not sure I got > the dimensions right. > > A problem I encountered and I was unable to fix is the popover getting > resized to fit the "Open" button when the download is finished. This looks fantastic! The blue selected background colour shouldn't be visible for any of the list rows, though. And a small bit of polish: Jakub has suggested that the open buttons could be smaller, so they don't dominate so much.
(In reply to Carlos Garcia Campos from comment #9) > Review of attachment 321732 [details] [review] [review]: > > ... I don't > understand why we have changed the open in folder button, that was > consistent with the cancel button, with a huge rectangular open button. The > downloads folder button can't replace the open in folder button of a > download. The advantage of the browse to action is that it not only opens > nautilus in the downloads folder but also selects the downloaded file. This > is very convenient and avoids having to search for the download in the > folder. Open in container action allows you to do whatever you want with the > download, moving to another folder, or just opening it, much better than our > open action. So, I think open in containing folder should be the most > important action for a download, more than open, that is still possible by > double clicking the download (I admit is a hidden, though). ... There were a few reasons why I suggested the new behaviour: First, it wasn't possible to directly open a downloaded file. Sure, you could go through the file manager, but this is an extra step, which is unwelcome in a lot of cases. It's pretty common to download files because they want to open them as quickly as possible. Second, the previous design didn't provide a way for the user to see all downloads. This is an essential feature if you are only showing a subset of everything that has been downloaded. And having multiple ways to open ~/Downloads seemed like micromanagement. Third, jumping to the downloaded file in ~/Downloads isn't all that useful. Users will generally be interested in recently downloaded stuff, which is at the top of the ~/Downloads folder, because it is sorted by recently modified.
(In reply to Allan Day from comment #13) > Third, jumping to the downloaded file in ~/Downloads isn't all that useful. > Users will generally be interested in recently downloaded stuff, which is at > the top of the ~/Downloads folder, because it is sorted by recently modified. It's a good point... I never noticed that, but it's true. Carlos, did you change the sort order of your downloads folder? I bet most users will never do that. We could perhaps make the Downloads Folder button select the most recently-downloaded file from the download popover, which would generally be what Carlos wants. It's a bit of a microoptimization, but seems harmless.
(In reply to Allan Day from comment #13) > (In reply to Carlos Garcia Campos from comment #9) > > Review of attachment 321732 [details] [review] [review] [review]: > > > > ... I don't > > understand why we have changed the open in folder button, that was > > consistent with the cancel button, with a huge rectangular open button. The > > downloads folder button can't replace the open in folder button of a > > download. The advantage of the browse to action is that it not only opens > > nautilus in the downloads folder but also selects the downloaded file. This > > is very convenient and avoids having to search for the download in the > > folder. Open in container action allows you to do whatever you want with the > > download, moving to another folder, or just opening it, much better than our > > open action. So, I think open in containing folder should be the most > > important action for a download, more than open, that is still possible by > > double clicking the download (I admit is a hidden, though). > ... > > There were a few reasons why I suggested the new behaviour: > > First, it wasn't possible to directly open a downloaded file. Just double click, I agree it's not easy to discover, but epiphany users are used to click the download button. > Sure, you > could go through the file manager, but this is an extra step, which is > unwelcome in a lot of cases. I agree, that's why you can double click. > It's pretty common to download files because > they want to open them as quickly as possible. Agree. > Second, the previous design didn't provide a way for the user to see all > downloads. I'm fine with the downloads button as I said in my previous comment, so I agree with this too. > This is an essential feature if you are only showing a subset of > everything that has been downloaded. And having multiple ways to open > ~/Downloads seemed like micromanagement. Open downloads folder and open download in containing folder are two different things for me, being the latter the most important one. > Third, jumping to the downloaded file in ~/Downloads isn't all that useful. It's very useful for me. But if I'm the only epiphany user who finds this useful, I'll keep my ephy patched. > Users will generally be interested in recently downloaded stuff, which is at > the top of the ~/Downloads folder, because it is sorted by recently modified. My download folder is /tmp, it's a mess to find anything there. I've always considered the downloads folder as something temporary, because in most of the cases I don't want to keep the downloaded file, and in the case I want to keep it, I always move it to another directory depending on what I've downloaded.
(In reply to Michael Catanzaro from comment #14) > (In reply to Allan Day from comment #13) > > Third, jumping to the downloaded file in ~/Downloads isn't all that useful. > > Users will generally be interested in recently downloaded stuff, which is at > > the top of the ~/Downloads folder, because it is sorted by recently modified. > > It's a good point... I never noticed that, but it's true. Carlos, did you > change the sort order of your downloads folder? I bet most users will never > do that. /tmp is my downloads folder > We could perhaps make the Downloads Folder button select the most > recently-downloaded file from the download popover, which would generally be > what Carlos wants. It's a bit of a microoptimization, but seems harmless. No it's not. I usually download several files, and then handle them individually. In most of the cases it's open what I want, so I just double click, but if I want to keep the file, I open it in the downloads folder to move it a an appropriate place, in that case, it doesn't need to be the last downloaded file.
(In reply to Carlos Garcia Campos from comment #16) > /tmp is my downloads folder I forgot this is configurable. > No it's not. I usually download several files, and then handle them > individually. In most of the cases it's open what I want, so I just double > click, but if I want to keep the file, I open it in the downloads folder to > move it a an appropriate place, in that case, it doesn't need to be the last > downloaded file. I don't have any good ideas here. I like Allan's design a lot more than the current popover, but it can't accommodate this workflow. :/ Maybe you could try changing the sort order of /tmp?
(In reply to Michael Catanzaro from comment #17) > (In reply to Carlos Garcia Campos from comment #16) > > /tmp is my downloads folder > > I forgot this is configurable. > > > No it's not. I usually download several files, and then handle them > > individually. In most of the cases it's open what I want, so I just double > > click, but if I want to keep the file, I open it in the downloads folder to > > move it a an appropriate place, in that case, it doesn't need to be the last > > downloaded file. > > I don't have any good ideas here. I like Allan's design a lot more than the > current popover, but it can't accommodate this workflow. :/ > > Maybe you could try changing the sort order of /tmp? That doesn't solve anything. I implemented the popover downloads at the very beginning of the cycle to get early feedback, I specifically asked about the discoverability of the open action, nobody has complained about it during the whole cycle, and now that we have entered the freeze you guys want to change not only the design but also the behavior. I don't think we should do that at this point, I'm fine with the UI improvements, but not behavior changes.
(In reply to Carlos Garcia Campos from comment #15) ... > I agree, that's why you can double click. Double-click is undiscoverable, particularly when there is no visual affordance like in this case. It might as well not exist. > > Users will generally be interested in recently downloaded stuff, which is at > > the top of the ~/Downloads folder, because it is sorted by recently modified. > > My download folder is /tmp, it's a mess to find anything there. Which is extremely unusual...
(In reply to Carlos Garcia Campos from comment #18) ... > I implemented the popover downloads at the very beginning of the cycle to > get early feedback, I specifically asked about the discoverability of the > open action, nobody has complained about it during the whole cycle, and now > that we have entered the freeze you guys want to change not only the design > but also the behavior. I don't think we should do that at this point, I'm > fine with the UI improvements, but not behavior changes. I'm sorry that the bug report came late. I thought we'd discussed the visual changes in Madrid back in December, and only realised that no improvements had landed until I did testing ahead of the UI freeze. (I don't tend to run epiphany master as a matter of course.) For what it's worth I do think that this would be worth a freeze break request - the popover looks quite rough currently.
Created attachment 321990 [details] Smaller button What about adding a right click menu with the "Open Containing Folder"/"Open in folder" item? It's common in other browsers. We can maybe also add a "Copy Download Link" in a subsequent patch if one item is not enough; it's another feature that you can usually find in that menu and one that I find myself using from time to time. I also made the open button a bit smaller (patch is not attached yet) and it looks pretty good.
(In reply to Iulian Radu from comment #21) > Created attachment 321990 [details] > Smaller button > > What about adding a right click menu with the "Open Containing Folder"/"Open > in folder" item? It's common in other browsers. We can maybe also add a > "Copy Download Link" in a subsequent patch if one item is not enough; it's > another feature that you can usually find in that menu and one that I find > myself using from time to time. Yes, I thought about that as well, but again right click menu is not discoverable, unless we use a button menu for the open button (that would justify using a different button for finished downloads) > I also made the open button a bit smaller (patch is not attached yet) and it > looks pretty good. Why not simply use the same button, just with a different icon? I still don't understand why we change the button for finished downloads.
(In reply to Allan Day from comment #20) > (In reply to Carlos Garcia Campos from comment #18) > ... > > I implemented the popover downloads at the very beginning of the cycle to > > get early feedback, I specifically asked about the discoverability of the > > open action, nobody has complained about it during the whole cycle, and now > > that we have entered the freeze you guys want to change not only the design > > but also the behavior. I don't think we should do that at this point, I'm > > fine with the UI improvements, but not behavior changes. > > I'm sorry that the bug report came late. I thought we'd discussed the visual > changes in Madrid back in December, Yes, but I don't remember we decided to change the behavior this way. > and only realised that no improvements > had landed until I did testing ahead of the UI freeze. (I don't tend to run > epiphany master as a matter of course.) > > For what it's worth I do think that this would be worth a freeze break > request - the popover looks quite rough currently. I'm not opposed to any UI improvement, I agree the current UI is not good and it's worth a freeze break, a different thing is removing features like open in containing folder.
We discussed this today, and agreed we will not remove the "open download in containing folder" option. One solution to this would be to keep the open button (with the folder icon) where it is now, make the entire EphyDownloadWidget clickable, and make clicking the widget trigger the open action.
Another solution would be to just try fitting two buttons into the popover.
(In reply to Carlos Garcia Campos from comment #23) > (In reply to Allan Day from comment #20) > > and only realised that no improvements > > had landed until I did testing ahead of the UI freeze. (I don't tend to run > > epiphany master as a matter of course.) > > > > For what it's worth I do think that this would be worth a freeze break > > request - the popover looks quite rough currently. > > I'm not opposed to any UI improvement, I agree the current UI is not good > and it's worth a freeze break, a different thing is removing features like > open in containing folder. Just dropping by to mention that I do agree that the popover design is much nicer that the current downloads bar, and having it will fix the issue of the Ephy window that cannot be made narrower when the downloads bar has a lot of elements... but I think the “Open containing folder” option is way too handy and useful to remove. One idea that comes to mind is having two side-by-side icons, one of them with a symbolic folder icon, and another with the “Open” text: this way the main “Open” action would still be featured more prominently and the “Open containing folder” action would take just a little space in each item of the popover. Just my two cents... :-)
As I said I'm not opposed to any change in the behavior if I'm the only one thinking differently, but at this point it's not possible to know if I'm the only one. What we know for sure is that the current popover has been there for the whole cycle and our user haven't complained about the open in containing folfer button or the not discoverability of the open action. So, let's improve the cosmetic issues, let's add the downloads folder button at the bottom, but we can't remove features at this point of the cycle. And next cycle we can discuss this with our users here or in the mailing list and do any changes we want.
Created attachment 322349 [details] [review] downloads-popover: Fix visual and layout issues The complete list of issues and solutions can be found in the bug report.
Created attachment 322351 [details] [review] downloads-popover: Fix visual and layout issues In the end: - I kept the new look without the open button; - rows cannot be selected anymore (there's no blue background color); - the download can now be opened with a single click instead of two;
Created attachment 322352 [details] [review] downloads-popover: Fix GtkListBoxRow not being destroyed When a download was removed (with the clear button or the remove button), the corresponding GtkListBoxRow containing the download widget was not removed. Remove the parent GtkListBoxRow containing the download widget.
Created attachment 322353 [details] [review] downloads-popover: Disappear more smoothly If the last download is being removed, hide the popover before destroying the last containing widget to avoid visual glitch.
Comment on attachment 322351 [details] [review] downloads-popover: Fix visual and layout issues Thank you very much Iulian, it looks really good, and it's more obvious now that the downloads are clickable. The only thing I still don't understand is why the button changes from circular to rectangular when the download finishes.
(In reply to Carlos Garcia Campos from comment #23) ... > I'm not opposed to any UI improvement, I agree the current UI is not good > and it's worth a freeze break, a different thing is removing features like > open in containing folder. I'm not going to apologise for recommending UI improvements as part of a design review.
(In reply to Carlos Garcia Campos from comment #22) ... > Why not simply use the same button, just with a different icon? I still > don't understand why we change the button for finished downloads. Because there isn't an established, commonly recognised icon for "open". See the section on "using icons in your user interface": https://developer.gnome.org/hig/stable/icons-and-artwork.html.en
(In reply to Michael Catanzaro from comment #24) > We discussed this today, and agreed we will not remove the "open download in > containing folder" option. That's disappointing - the current design is pretty bad.
(In reply to Allan Day from comment #33) > (In reply to Carlos Garcia Campos from comment #23) > ... > > I'm not opposed to any UI improvement, I agree the current UI is not good > > and it's worth a freeze break, a different thing is removing features like > > open in containing folder. > > I'm not going to apologise for recommending UI improvements as part of a > design review. There's nothing for which you should apologize, your feedback is always welcome, the only thing I'm saying is that this time it's too late to remove features and that we can discuss your suggestions with ephy users for the next release cycle.
Created attachment 322358 [details] [review] downloads-popover: Fix visual and layout issues > The only thing I still don't understand is why the button changes > from circular to rectangular when the download finishes. A misunderstanding on my part. You are right, the button should be circular.
Comment on attachment 322352 [details] [review] downloads-popover: Fix GtkListBoxRow not being destroyed Thanks!
Review of attachment 322353 [details] [review]: Thanks! ::: lib/widgets/ephy-downloads-popover.c @@ +96,3 @@ children = gtk_container_get_children (GTK_CONTAINER (popover->downloads_box)); + + if (g_list_length (children) == 1) Please, add a comment here explaining why we return early when there's only one download left.
Comment on attachment 322358 [details] [review] downloads-popover: Fix visual and layout issues Thank you very much. Please, ask for freeze break and push it once you have the approval.
Created attachment 322368 [details] [review] downloads-popover: Disappear more smoothly Should have been gtk_widget_hide () instead of return. Not sure how I missed that. I have added a comment anyway.
FWIW I would prefer the Open text button and remove the 'open in containing folder' action myself, as Allan's design is cleaner and I'm really suspicious that many users will use 'open in containing folder', but I agree it's too late for this cycle. I tested Iulian's patches and they're an uncontroversial improvement over the current design, so let's get them in.
Comment on attachment 322358 [details] [review] downloads-popover: Fix visual and layout issues >+ popover->downloads_folder_button = gtk_button_new_with_label (_("Downloads Folder")); >+ popover->clear_button = gtk_button_new_with_label (_("Clear All")); Didn't compile hence just wondering: Is it intentional to not set any mnemonics for these buttons?
No, it's an oversight, good catch: Iulian, you'll want to add mnemonics. I actually think we don't need the Downloads Folder button at all, since, for better or for worse, we have individual open in downloads folder buttons.
*** Bug 760695 has been marked as a duplicate of this bug. ***
Created attachment 322501 [details] [review] downloads-popover: Fix visual and layout issues I have removed the Downloads Folder button in the end. I have also added the missing mnemonic to the Clear All button.
Review of attachment 322368 [details] [review]: ++
Review of attachment 322501 [details] [review]: OK. Thanks for updating this again and again! ::: lib/widgets/ephy-download-widget.c @@ +429,3 @@ gtk_widget_set_margin_start (widget->action_button, 10); gtk_style_context_add_class (gtk_widget_get_style_context (widget->action_button), + "circular"); I didn't know you could do this!
To be clear: when I set accepted-commit-now, I normally expect you to push the patches. In this case, I happen to be testing downloads now, so I'll push them as it's convenient for me.
Attachment 322352 [details] pushed as edfd6c4 - downloads-popover: Fix GtkListBoxRow not being destroyed Attachment 322368 [details] pushed as cb048f1 - downloads-popover: Disappear more smoothly Attachment 322501 [details] pushed as 0fd589c - downloads-popover: Fix visual and layout issues