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 761158 - Filter tool doesn't reflect the previously applied filter when re-activated
Filter tool doesn't reflect the previously applied filter when re-activated
Status: RESOLVED FIXED
Product: gnome-photos
Classification: Applications
Component: general
3.19.x
Other All
: Normal normal
: ---
Assigned To: GNOME photos maintainer(s)
GNOME photos maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-01-27 04:48 UTC by Umang Jain
Modified: 2016-02-05 10:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tool-filter: Reflect previously applied filter(if any) (2.77 KB, patch)
2016-01-29 08:06 UTC, Umang Jain
committed Details | Review
tool-filter-button, tool-filters: Select previous filter (if any) (3.30 KB, patch)
2016-02-05 10:20 UTC, Debarshi Ray
committed Details | Review
tool-filter-button, tool-filters: Select previous filter (if any) (3.26 KB, patch)
2016-02-05 10:28 UTC, Debarshi Ray
committed Details | Review

Description Umang Jain 2016-01-27 04:48:32 UTC
Ideally, when you apply a filter and click on "Done", it should apply the filter and exit the edit palette. When edit palette is re-activated, the "Filter" tool should reflect the filter which has been applied earlier but currently, it does not do so.
Comment 1 Umang Jain 2016-01-29 08:06:29 UTC
Created attachment 320003 [details] [review]
tool-filter: Reflect previously applied filter(if any)
Comment 2 Debarshi Ray 2016-02-05 10:19:17 UTC
Review of attachment 320003 [details] [review]:

Thanks for the nice patch, Umang. It works perfectly. Some cosmetic things:

::: src/photos-tool-filter-button.h
@@ +49,3 @@
                                                                          GtkWidget *image);
 
+void                   photos_tool_filter_button_set_activated_button   (PhotosToolFilterButton *self);

It would be slightly better to call it photos_tool_filter_button_set_active to match gtk_toggle_button_set_active. We try to mimic the GtkButton, GtkToggleButton API whenever applicable.

::: src/photos-tool-filters.c
@@ +123,3 @@
+                                      NULL))
+  {
+    button = g_list_nth_data (self->buttons, (guint) preset);

It would be more robust to iterate over the list of buttons and compare the presets. The fact we order the buttons in increasing order of the preset enum is just a cosmetic thing. Things shouldn't break if we shuffle the buttons around.
Comment 3 Debarshi Ray 2016-02-05 10:20:05 UTC
Created attachment 320491 [details] [review]
tool-filter-button, tool-filters: Select previous filter (if any)

Made the above adjustments and pushed.
Comment 4 Debarshi Ray 2016-02-05 10:28:58 UTC
Created attachment 320492 [details] [review]
tool-filter-button, tool-filters: Select previous filter (if any)