GNOME Bugzilla – Bug 733214
Use a fallback name in the header bar during preview
Last modified: 2014-12-16 14:01:48 UTC
When I'm viewing photos, the headerbar typically lacks a heading. It should always have one - both to provide context to the window and the content item on display. If the photo doesn't have a name set, you can always use the file name.
I think we can to add title and subtitle. Subttile - the patch of the file Title - the name of the file, if have not - the filename itself
If there is no title set for an image ,can we use its path name consisting of file name as well.?
The path seems too much. The goal of the bug is to provide photos with some identity, not to tie it in with the file system - which is something that photos deliberately tries to avoid.
Okay. So i have got two options.Either using just the file name not the entire path and the second option is giving untitled photos header as something like untitled-1,untitled 2,...so on Which of the option need to be implemented?
Created attachment 283219 [details] [review] Now header bar has title for each and every image being previewed.
Review of attachment 283219 [details] [review]: I think you may want to set your git config email-id first. It says amisha@localhost.localdomain. May be set it to something real ? |git config --global user.email "your_email@example.com"| should do the trick. There is a lot of scope of improvement in the commit message. First of all the title of the commit message should be as short as possible. So you can try to short this one which I find it really long. You can explain things after leaving one empty line after the title of the commit message which you have done correctly in this case but again there is no need to prefix it with "Description:" Also there is extra ":wq" at the end of the commit message. You can have a look at |git log| and get a hint from other commit messages.
Created attachment 283362 [details] [review] Context for every image preview is shown on header bar
Review of attachment 283362 [details] [review]: There is still some problem with your git config. The Author Name says "New Author Name" which I think you would like to change to your Real name.
Review of attachment 283362 [details] [review]: There are few coding convention issues I noticed. 1. There should be space when calling a function. Eg: In your call to strcmp(), your code looks like : strcmp(primary, "") It should be like : strcmp (primary, "") 2. Also notice the space between the arguments to functions. Eg: One of the lines in your patch says : gtk_header_bar_set_title (GTK_HEADER_BAR (priv->toolbar),g_basename(lastname)); but it should be like gtk_header_bar_set_title (GTK_HEADER_BAR (priv->toolbar), g_basename (lastname)); Notice the spaces. Correct it for whole of your patch. ::: src/photos-main-toolbar.c @@ +82,3 @@ gboolean selection_mode; gchar *primary = NULL; + gchar *lastname = NULL; May be we can rename this to something more informative and intuitive. @@ +131,2 @@ item = photos_base_manager_get_active_object (priv->item_mngr); + if (item != NULL){ It would be better if you start the "if" block braces from the next line just to be consistent with the rest of the codebase. Like this : if (item != NULL) { ... Follow the same thing for the rest of your patch. Same with else blocks. @@ +145,3 @@ + ext = strchr(g_basename(lastname),'.'); + *ext='\0'; + gtk_header_bar_set_title (GTK_HEADER_BAR (priv->toolbar),g_basename(lastname)); 1. There is already a function in photos-utils.h i.e photos_utils_filename_strip_extension. I think it would be better if you use that instead. 2. g_basename is deprecated since version 2.2 You should use g_path_get_basename () instead. 3. lastname is getting leaked. May be you forgot to g_free it.
Created attachment 283383 [details] [review] Context for every image preview is shown on header bar
Review of attachment 283383 [details] [review]: ::: src/photos-main-toolbar.c @@ +44,3 @@ #include "photos-search-context.h" #include "photos-selection-controller.h" +#include "photos-utils.h" You removed an extra line while adding this #include. Use a new line for adding this new #include. @@ -82,3 @@ gboolean selection_mode; gchar *primary = NULL; - gchar *lastname = NULL; Well, this shows that your patch is not rebased against master. It would be great if you rebase your work against master and submit your work as one single patch.
As I was telling Pranav at the BoF during GUADEC, this sounds a bit like the opposite of what we did in bug 688562 I guess the difference would be that we are only going to use the filename fallback during preview, not in the overview?
Review of attachment 283383 [details] [review]: ::: src/photos-main-toolbar.c @@ +146,3 @@ + { + gtk_header_bar_set_title (GTK_HEADER_BAR (priv->toolbar), + photos_utils_filename_strip_extension (g_path_get_basename (filename))); Have you tried this with any online sources? eg., Flickr. In those cases the filename in URI is not human readable, but machine generated. It is not visible even in Flickr's web UI. I am not sure it makes much sense to use it in our UI. We might want to let each source define its own fallback to the title.
By the way, thanks for working on this patch, Amisha. Much appreciated.
(In reply to comment #12) > As I was telling Pranav at the BoF during GUADEC, this sounds a bit like the > opposite of what we did in bug 688562 I guess the difference would be that we > are only going to use the filename fallback during preview, not in the > overview? Yes, this is only about the filename fallback during preview.
Thanks. Actually i am not able to get what you are trying to convey from your comments regarding online sources.
Created attachment 285252 [details] [review] Context for every image preview is shown on header bar
Review of attachment 285252 [details] [review]: As rishi suggested in Comment 13, better approach would be to let each source define its own fallback to the title.
There are a few issues here. First of all, you might notice that filenames have started to appear all over the UI, which was not the case before. That is due to a change in tracker, that will soon be reverted. See bug 740425 That aside, what should we use as the fallback in the header bar for remote content? eg., a Facebook URI can look like https://www.facebook.com/album.php?fbid=500314756654329&id=100000274286477&aid=118687 - utterly meaningless to a human. Could we do something like "Facebook - 2nd January 2013" instead? For local content, I think we can just live with the filename as Allan suggested. Possibly after removing the extension as we do in gnome-documents.
(In reply to comment #19) > > That aside, what should we use as the fallback in the header bar for remote > content? eg., a Facebook URI can look like > https://www.facebook.com/album.php?fbid=500314756654329&id=100000274286477&aid=118687 > - utterly meaningless to a human. Could we do something like "Facebook - 2nd > January 2013" instead? I can see two cases here. One for remote albums and one for remote photos. We can easily have the name of the album in the first case. Eg: It can be "[Facebook] Cover Photos" for above mentioned link. For latter case, I was thinking of incorporating the description of the photos somehow in gnome-photos. Eg: It can be "Lake Meiko" for this photos : https://www.facebook.com/photo.php?fbid=500314766654328&set=a.500314756654329.118687.100000274286477&type=3&theater as this string is mentioned in the description of this photos. But again, this string can be really huge sometimes which might not be desirable and sometimes it can be nothing. We can just fallback to the timestamp of the photos in these undesirable cases ?
(In reply to comment #20) > (In reply to comment #19) > I can see two cases here. One for remote albums and one for remote photos. We > can easily have the name of the album in the first case. Eg: It can be > "[Facebook] Cover Photos" for above mentioned link. Albums are not a problem. They always have a title. > For latter case, I was > thinking of incorporating the description of the photos somehow in > gnome-photos. If a Facebook photo has a description, then it already has a title. Think of a photograph on Flickr such as this one: https://www.flickr.com/photos/debarshiray/9572984119/ It has neither title nor description. So we need to create some sensible string.
Created attachment 291738 [details] [review] Use a fallback name in the header bar when there is no nie:title
Review of attachment 285252 [details] [review]: It has been quite a while since this patch was updated and it is not usable in its current form.
(In reply to comment #22) > Created an attachment (id=291738) [details] [review] > Use a fallback name in the header bar when there is no nie:title MediaServer items have nie:title always set to DisplayName property provided by the DMS which is a friendly name set by the DMS from its filename. API docs of dleyna-server says : "DisplayName is the friendly name the DMS should associate with the file and FilePath is a full path to the file." I have not found any cases as of now where DisplayName is not set by the DMS.
(In reply to comment #24) > (In reply to comment #22) > > Created an attachment (id=291738) [details] [review] [details] [review] > > Use a fallback name in the header bar when there is no nie:title > > MediaServer items have nie:title always set to DisplayName property provided by > the DMS which is a friendly name set by the DMS from its filename. > > API docs of dleyna-server says : > > "DisplayName is the friendly name the DMS should associate with > the file and FilePath is a full path to the file." > > I have not found any cases as of now where DisplayName is not set by the DMS. Thanks for clearing that up, Pranav.
*** Bug 734767 has been marked as a duplicate of this bug. ***