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 793841 - added scrubbing functionality to directory items
added scrubbing functionality to directory items
Status: RESOLVED FIXED
Product: shotwell
Classification: Other
Component: ux
unspecified
Other All
: Normal enhancement
: 0.30
Assigned To: Shotwell Maintainers
Shotwell Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-02-26 13:47 UTC by carsten.csiky
Modified: 2018-07-15 15:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch, that enables that functionality (3.01 KB, patch)
2018-02-26 13:47 UTC, carsten.csiky
none Details | Review

Description carsten.csiky 2018-02-26 13:47:42 UTC
Created attachment 368942 [details] [review]
Patch, that enables that functionality

This changes the behaviour of EventItems when the user moves the mouse over them, scrolling through the images contained.
It should look like this: [example](https://csicar.de/1.gif).

I am not sure, if all the necessary places have been updated and if adding the method `handle_mouse_motion` is fine, If not I would appreciate feedback.

Todos for me (if there even is interest in merging):
- [ ] switch back to the primary_image if the mouse leaves an EventItem
- [ ] comments for code
Comment 1 Jens Georg 2018-02-28 16:32:57 UTC
Does it change the event picture permanently?
Comment 2 carsten.csiky 2018-02-28 16:35:46 UTC
no, it does not
Comment 3 carsten.csiky 2018-02-28 16:50:07 UTC
If there is interest in having this feature in Shotwell. I will make the shown Image change back to the primary image when the mouse leaves an EventItem
Comment 4 Jens Georg 2018-02-28 17:17:43 UTC
Review of attachment 368942 [details] [review]:

Did you try it with very large events? it already looks quite flickery with ~55 pictures

::: src/events/EventDirectoryItem.vala
@@ +170,3 @@
+        int element_index = (int) Math.round( (double) x / width * (event.get_media_count() - 1) );
+        unowned MediaSource media = event.get_media().to_array()[element_index];
+

The continous conversion to array could turn out to be annoying for very large collections, but I currently see no easier way
Comment 5 Jens Georg 2018-02-28 17:23:40 UTC
(In reply to carsten.csiky from comment #3)
> If there is interest in having this feature in Shotwell. I will make the
> shown Image change back to the primary image when the mouse leaves an
> EventItem

That would be a plus, because that's what confused me.

From my POV I would like to take it in, after feature freeze (which Shotwell is currently in, in alignment with GNOME schedule)
Comment 6 carsten.csiky 2018-02-28 17:28:48 UTC
(In reply to Jens Georg from comment #4)
> Review of attachment 368942 [details] [review] [review]:
> 
> Did you try it with very large events? it already looks quite flickery with
> ~55 pictures
> 
> ::: src/events/EventDirectoryItem.vala
> @@ +170,3 @@
> +        int element_index = (int) Math.round( (double) x / width *
> (event.get_media_count() - 1) );
> +        unowned MediaSource media =
> event.get_media().to_array()[element_index];
> +
> 
> The continous conversion to array could turn out to be annoying for very
> large collections, but I currently see no easier way

I quite dislike that as well. The only way I have seen for improving that would be caching the array. Is there a better way?
Comment 7 carsten.csiky 2018-02-28 17:39:28 UTC
(In reply to Jens Georg from comment #4)
> Review of attachment 368942 [details] [review] [review]:
> 
> Did you try it with very large events? it already looks quite flickery with
> ~55 pictures
> 
> ::: src/events/EventDirectoryItem.vala
> @@ +170,3 @@
> +        int element_index = (int) Math.round( (double) x / width *
> (event.get_media_count() - 1) );
> +        unowned MediaSource media =
> event.get_media().to_array()[element_index];
> +
> 
> The continous conversion to array could turn out to be annoying for very
> large collections, but I currently see no easier way

About the flickering: Do you mean "flickery" caused by the users mouse movement or by bad performance.

In terms of performance, I did not experience any problems with events up to ~400 pictures (testing on an i5-5200U, 12G Ram and SSD, which is definitely above what the minimum requirement should be for having this feature work properly)
Comment 8 carsten.csiky 2018-02-28 19:14:54 UTC
I created a basic version, that implements the "show thumbnail on mouse leave" like this: https://github.com/GNOME/shotwell/compare/master...csicar:master.diff

Needed refactoring in that patch: 
- I changed the name of "highlighted" in Page.vala to "current_hovered_item", since it is now also used for this purpose. 
- Created "handle_mouse_leave()" and "handle_mouse_enter()" and use there methods for the highlight() and unhighlight() methods
Comment 9 Jens Georg 2018-07-03 13:54:55 UTC
I'm sorry, I somewhat forgot about this. I will take a look. With flickery I meant the mouse movement when the event carries many photos
Comment 10 carsten.csiky 2018-07-04 09:37:20 UTC
The Solution could be limiting the minimum hover-width, where an image will be displayed. Like this: https://gitlab.gnome.org/csicar/shotwell/compare/image-scrubbing...image-non-flickery-scrubbing
Comment 11 Jens Georg 2018-07-05 21:49:50 UTC
Cool. I think % 15 is enough.

Can you squash the refactor commit into the relevant previous commits and open a merge request?
Comment 12 carsten.csiky 2018-07-06 06:52:33 UTC
here is the merge request https://gitlab.gnome.org/GNOME/shotwell/merge_requests/3