After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 741593 - No pagination buttons when previewing search results
No pagination buttons when previewing search results
Status: RESOLVED FIXED
Product: gnome-photos
Classification: Applications
Component: general
3.14.x
Other All
: Normal normal
: ---
Assigned To: GNOME photos maintainer(s)
GNOME photos maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-12-16 12:52 UTC by Jakub Steiner
Modified: 2015-10-09 11:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gnome-photos: fixed navigation buttons not showing on search result (1.19 KB, patch)
2015-04-21 23:29 UTC, Julio Ordonez V
none Details | Review
photos-searchbar: do not set search string to empty string (1003 bytes, patch)
2015-04-22 01:26 UTC, Julio Ordonez V
none Details | Review
photos-searchbar: block search change when picking an item (2.00 KB, patch)
2015-04-29 23:15 UTC, Julio Ordonez V
none Details | Review
Turn the PhotosTrackerController sub-classes into extensions (9.41 KB, patch)
2015-10-07 18:03 UTC, Debarshi Ray
committed Details | Review
tracker-controller: Consolidate the path to submitting a query (2.36 KB, patch)
2015-10-07 18:04 UTC, Debarshi Ray
committed Details | Review
Let users page through search results in the preview (6.75 KB, patch)
2015-10-07 18:06 UTC, Debarshi Ray
committed Details | Review

Description Jakub Steiner 2014-12-16 12:52:31 UTC
When I select a 'local' source filter form the search bar, clicking on any photo thumbnail opens the image view but doesn't provide a mean to page to the next one in the set.
Comment 1 Pranav Kant 2015-01-02 20:44:34 UTC
Similar situation can be reproduced when we filter by Type (Eg. selecting 'Favorite' or 'Photos' in Type filter).
Comment 2 Julio Ordonez V 2015-04-01 22:23:41 UTC
Can I work on this one?
Comment 3 Debarshi Ray 2015-04-03 08:24:31 UTC
(In reply to Julio Ordonez V from comment #2)
> Can I work on this one?

Sure, go ahead.
Comment 4 Julio Ordonez V 2015-04-09 01:59:52 UTC
do you have any clues as to which functions should I be looking into? I've been studying the code but still quite not get it...
Comment 5 Alessandro Bono 2015-04-10 19:29:46 UTC
I think that it happens because priv->current_path in photos_preview_nav_buttons_update_visibility (file photos-preview-nav-buttons.c) is NULL. So it sets priv->enable_prev and priv->enable_next on FALSE. The priv->current_path is setted in photos_preview_nav_buttons_set_model by gtk_tree_model_filter_convert_child_path_to_path function. The documentation says: "If child_path isn’t a valid path on the child model or points to a row which is not visible in filter, then NULL is returned.". I looked at the model code but I didn't find nothing more.
Comment 6 Debarshi Ray 2015-04-15 07:33:48 UTC
gtk_tree_model_filter_convert_child_path_to_path returns NULL because the child model has been cleared and is empty.

When you click an item photos_embed_active_changed hides the searchbar, which clears the search string. That triggers a new query and the model is cleared by the time it reaches PreviewView.
Comment 7 Julio Ordonez V 2015-04-18 21:40:10 UTC
What would be the best approach to solve the issue, should the model be saved somewhere? 
I was checking and after performing a search even when you click on some item photos_embed_active_changed never got called. I put some g_prints around and I did not see it when I click on some item in the search result.
Comment 8 Julio Ordonez V 2015-04-21 23:29:53 UTC
Created attachment 302098 [details] [review]
gnome-photos: fixed navigation buttons not showing on search result

navigation buttons where not showing after you clicked on a search result
item, photos_embed_active_changed was hidding the searchbar, clearing the
search string and triggering a new search. Fixed it by adding a condition
to check if the search was saved before calling g_action_change_state.
Comment 9 Julio Ordonez V 2015-04-22 01:26:47 UTC
Created attachment 302115 [details] [review]
photos-searchbar: do not set search string to empty string

setting the search string to an empty string was triggering a new search after
the searchbar was hidden, therefore the navigation buttons where not showing up
after selecting an item on the search result.
Comment 10 Julio Ordonez V 2015-04-26 16:43:28 UTC
Hi everyone,

So the previous patches don't work because the searchbar is not hidden or the search string is not cleared when you hide the searchbar. I am thinking on blocking signals from photos_embed_active_changed(), but I do not know how to do it.

 According to the docs:

g_signal_handlers_block_by_func(instance, func, data)

where instance would be the search controller, function the callback function and data

data

The closure data of the handlers' closures.

I guess it would be the data that you must pass to the function. I've seen something like that in the code for example:

g_signal_handlers_block_by_func (priv->srch_cntrlr, photos_embed_search_changed, self);

so I wanted to block a signal that is emitted when the search controller string is changed, since I don't know if I would be able to block a signal in the searchbar since I am on a different compilation unit and I do not hold a reference to the searchbar or the data that should be pass to the function.

but there is something else : in photos_search_controller_class_init
I just see

 signals[SEARCH_STRING_CHANGED] = g_signal_new ("search-string-changed",
G_TYPE_FROM_CLASS (class),
G_SIGNAL_RUN_LAST,
                                                  G_STRUCT_OFFSET (PhotosSearchControllerClass,
search_string_changed),
                                                  NULL, /*accumulator */
                                                  NULL, /*accu_data */
g_cclosure_marshal_VOID__STRING,
                                                  G_TYPE_NONE,
                                                  1,
                                                  G_TYPE_STRING);

and in the photos_search_controller_set_string() function I can see this

  g_signal_emit (self, signals[SEARCH_STRING_CHANGED], 0, priv->str);

so I am not sure how would I block that signal, since it looks different from the other example, where the signal is connected to a function, if I use the function g_signal_handlers_block_by_func() what do I need to use in func since there doesn't seem to be a function associated with the signal, and last what should I use in the data argument. Maybe I am not doing the right thing or there is another way to do it.

I would appreciate your advice,


Thanks,

Julio Ordonez
Comment 11 Julio Ordonez V 2015-04-29 23:15:51 UTC
Created attachment 302617 [details] [review]
photos-searchbar: block search change when picking an item

setting the search string to an empty string was triggering a new search after
the searchbar was hidden when clicking on a search result item, therefore
the navigation buttons where not showing up. Fixed by blocking the function
photos_searchbar_search_changed
Comment 12 Debarshi Ray 2015-05-06 05:22:30 UTC
I am confused ...

(In reply to Julio Ordonez V from comment #10)
> So the previous patches don't work

Are the first and second patches still relevant or can they be marked as obsolete?
Comment 13 Julio Ordonez V 2015-05-06 05:39:35 UTC
they are both obsolete, sorry
Comment 14 Debarshi Ray 2015-05-06 06:10:40 UTC
(In reply to Julio Ordonez V from comment #10)
>  According to the docs:
> 
> g_signal_handlers_block_by_func(instance, func, data)
> 
> where instance would be the search controller, function the callback
> function and data
> 
> data
> 
> The closure data of the handlers' closures.
> 
> I guess it would be the data that you must pass to the function. I've seen
> something like that in the code for example:
> 
> g_signal_handlers_block_by_func (priv->srch_cntrlr,
> photos_embed_search_changed, self);

In this case the signal is still emitted, but this particular signal handler (ie. photos_embed_search_changed) is not invoked. Other handlers connected to this signal that have not been blocked will still get called.

> and in the photos_search_controller_set_string() function I can see this
> 
>   g_signal_emit (self, signals[SEARCH_STRING_CHANGED], 0, priv->str);
> 
> so I am not sure how would I block that signal

You can't really "block" the g_signal_emit unless you add your own API/flag to the class that owns the signal to avoid the emission.
Comment 15 Debarshi Ray 2015-05-06 06:58:50 UTC
Here is an idea. How about "freezing" the tracker controllers when we move away from the overview?
Comment 16 Debarshi Ray 2015-10-07 18:03:47 UTC
Created attachment 312841 [details] [review]
Turn the PhotosTrackerController sub-classes into extensions
Comment 17 Debarshi Ray 2015-10-07 18:04:13 UTC
Created attachment 312842 [details] [review]
tracker-controller: Consolidate the path to submitting a query
Comment 18 Debarshi Ray 2015-10-07 18:06:23 UTC
Created attachment 312843 [details] [review]
Let users page through search results in the preview
Comment 19 Debarshi Ray 2015-10-07 18:07:07 UTC
Patches also in wip/rishi/filter-freeze.
Comment 20 Debarshi Ray 2015-10-08 16:55:34 UTC
The other option is to break the connection between the "search" action and the singletons making up the search state (eg., SourceManager, SearchController, etc.), but that looks more complicated than cutting off the queries.
Comment 21 Debarshi Ray 2015-10-09 11:07:53 UTC
Pushed to master and gnome-3-16.