| Summary: | Allow a simpler way to place a photo in full-screen using overlay [patch] | ||
|---|---|---|---|
| Product: | [Applications] digikam | Reporter: | Luca Carlon <carlon.luca> |
| Component: | Usability-Overlay | Assignee: | Digikam Developers <digikam-bugs-null> |
| Status: | RESOLVED FIXED | ||
| Severity: | wishlist | CC: | alexander.fieroch, caulier.gilles, metzpinguin, timetre, yan |
| Priority: | NOR | ||
| Version: | 4.13.0 | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | http://commits.kde.org/digikam/e65663863acf06671adc3e719d6f75ce3fbb4fa9 | Version Fixed In: | 4.14.0 |
| Sentry Crash Report: | |||
| Attachments: |
Patch that adds a new "fullscreen mode" and binds it the the double click in the preview.
Patch that applies to 81a53fe to implement fullscreen. Patch that applies to 274d73d. Result. Patch that applies to 19da552. Full Screen Overlay patch 2 Full Screen Overlay patch 3 |
||
|
Description
Luca Carlon
2014-09-14 21:55:08 UTC
Created attachment 88703 [details]
Patch that adds a new "fullscreen mode" and binds it the the double click in the preview.
This patch adds the new "fullscreen mode" and also considers it some kind of "manual slideshow". On IRC this came up to be inappropriate as not to be considered an actual slideshow. So that part may be removed.
I have to say the patch does not completely satisfy me but... you guys may help improving it... Luca, With current implementation from git/master, a lots of changes have been done to SlideShow. You patch cannot be applied and tested. Can you update it and adapt your code to new implementation ? Thanks in advance Gilles Caulier Luca, I agree with you. Have a look at https://bugs.kde.org/show_bug.cgi?id=280133 Created attachment 89690 [details]
Patch that applies to 81a53fe to implement fullscreen.
Can someone review this patch before new modifications make it impossible to apply again? Please sync the patch with current implementation from git/master. Branching code to port to KF5 will be done after 4.6.0 release (https://techbase.kde.org/Projects/Digikam/CodingSprint2014#KDE_Framework_Port) Luca, Any change to have this patch updated with last implementation from git/master ? Gilles Caulier A third implementation of the same feature?! Impossible to apply again? Luca, I don't understand what you mean exactly (:=))... ? Gilles Caulier Luca, no response ? Gilles Caulier I already wrote this patch two times. Where would you like this applied? git master? yes git/master... Gilles Caulier Created attachment 94106 [details]
Patch that applies to 274d73d.
Could you please have a look at the new patch? It seems to work as expected for me. Any comment on this patch? Should I change something? The idea for a quick full screen mode I like that. Only I would not like to lose the left mouse button click, to return to the icon view. Instead of double click, I would for a overlay button in the preview or an additional entry in the slideshow menu. We'll see what Gilles says. Improvements to the patch: - Slideshow->With All Sub-Albums is not working (builder->run() is missing) - Unused variable delay - Remove qDebug Maik The ide The idea to use an overlay is fine for me. This want mean to support this kind of display mode switch over icon view and thumbbar ? If yes, i'm agree... Gilles Mmh... ok, I can probably do that. But... am I the only one who feels more natural that double click "opens" to fullscreen? On a desktop computer I typically see that double clicks (or single clicks) "open" the target, while the escape button or whatever else "close" or "go back". So from the icon view I click and I "open" the image in the preview, when I click (or better double click) the preview I "open" the image to fullscreen... while escape, the button in the toolbar and whatever else "go back" to preview and again to the icon view. By having a look around i see iPhoto actually does what you proposed, while gwenview does more or less what I said. So, if you confirm you want an overlay button I can try to modify and fix the patch according to the behaviour you requested. overlay button -> yes, confirmed... Gilles Caulier Another note : All icon view overlay are optional and can be turned on/of to Setup/Album View tab from Config dialog. Your new overlay must be configurable. Gilles Caulier Hello, sorry, I have a doubt about the decision. Maik said "[...] I would for a overlay button in the preview [...]" but then Gilles said "All icon view overlay are optional [...]". So, where is this overlay button supposed to be placed? In the icon view (which is marked as thumbnails in the toolbar button) or in the preview view (marked as preview image in the toolbar)? The overlay is over each icon-view item, as for ex the GPS indicator overlay, which can be enable or disable into Album-view setup page. This has nothing to see with toolbar. Gilles Caulier Created attachment 94311 [details]
Result.
Before I go on with the implementation, is this the result you expect? Of course I need a proper icon, maybe I can use the one used in fullscreen.
yes it's the right way... Gilles Caulier What icon should I use for the overlay? The same icon that one used with Slideshow button from main toolbar. Gilles Caulier Created attachment 94425 [details]
Patch that applies to 19da552.
Can someone review this patch? It includes new setup to show/hide the overlay, the overlay itself in icon view, the new "slide show" mode and a similar button in preview. Created attachment 94426 [details] Full Screen Overlay patch 2 I review patch and fixed some details about overlay icon size which follow thumbnails size, since we have integrated changes from bug #261568. Maik, please try this patch. If all is fine for you, let's go to apply it to git/master Gilles I have made still polishes. I find the full screen icon is not suitable for round overlay buttons. The corners of the icon are over. Possibly the "media-playback-start" button? Maik I faced the same problem when i test the patch. I don't find a right icon (rounded) from Oxygen sets... And yes, perhaps "media-playback-start" button is fine. As i can see, Gwenview use it with a similar action from menu. Gilles Created attachment 94438 [details]
Full Screen Overlay patch 3
Git commit e65663863acf06671adc3e719d6f75ce3fbb4fa9 by Maik Qualmann. Committed on 06/09/2015 at 20:47. Pushed by mqualmann into branch 'master'. apply patch #94438 from Luca Carlon to allow a simpler way to place a photo in full-screen FIXED-IN: 4.14.0 M +1 -0 CMakeLists.txt M +2 -1 NEWS M +17 -0 app/items/digikamimageview.cpp M +2 -0 app/items/digikamimageview.h M +27 -0 app/items/digikamimageview_p.cpp M +3 -0 app/items/digikamimageview_p.h A +122 -0 app/items/overlays/imagefsoverlay.cpp [License: GPL (v2+)] A +87 -0 app/items/overlays/imagefsoverlay.h [License: GPL (v2+)] M +2 -0 app/settings/applicationsettings.cpp M +3 -0 app/settings/applicationsettings.h M +10 -0 app/settings/applicationsettings_iconview.cpp M +3 -0 app/settings/applicationsettings_p.cpp M +2 -0 app/settings/applicationsettings_p.h M +25 -5 app/utils/slideshowbuilder.cpp M +12 -0 app/utils/slideshowbuilder.h M +33 -2 app/views/digikamview.cpp M +2 -0 app/views/digikamview.h M +9 -0 app/views/imagepreviewview.cpp M +1 -0 app/views/imagepreviewview.h M +3 -0 app/views/stackedview.cpp M +1 -0 app/views/stackedview.h M +10 -1 utilities/setup/setupalbumview.cpp M +8 -0 utilities/slideshow/slideshowsettings.h M +7 -4 utilities/slideshow/slidetoolbar.cpp http://commits.kde.org/digikam/e65663863acf06671adc3e719d6f75ce3fbb4fa9 Git commit 6d9bf9ffc869424075f108bfad57ca5ed17a0310 by Gilles Caulier. Committed on 08/09/2015 at 08:49. Pushed by cgilles into branch 'frameworks'. backport commit #e65663863acf06671adc3e719d6f75ce3fbb4fa9 from git/mùaster to frameworks branch M +1 -0 app/CMakeLists.txt M +17 -0 app/items/digikamimageview.cpp M +2 -0 app/items/digikamimageview.h M +27 -0 app/items/digikamimageview_p.cpp M +3 -0 app/items/digikamimageview_p.h A +125 -0 app/items/overlays/imagefsoverlay.cpp [License: GPL (v2+)] A +85 -0 app/items/overlays/imagefsoverlay.h [License: GPL (v2+)] M +25 -5 app/utils/slideshowbuilder.cpp M +18 -2 app/utils/slideshowbuilder.h M +34 -3 app/views/digikamview.cpp M +2 -0 app/views/digikamview.h M +13 -2 app/views/imagepreviewview.cpp M +1 -0 app/views/imagepreviewview.h M +3 -0 app/views/stackedview.cpp M +1 -0 app/views/stackedview.h M +2 -0 libs/settings/applicationsettings.cpp M +4 -1 libs/settings/applicationsettings.h M +10 -0 libs/settings/applicationsettings_iconview.cpp M +3 -0 libs/settings/applicationsettings_p.cpp M +2 -0 libs/settings/applicationsettings_p.h M +10 -1 utilities/setup/setupalbumview.cpp M +8 -0 utilities/slideshow/slideshowsettings.h M +5 -2 utilities/slideshow/slidetoolbar.cpp http://commits.kde.org/digikam/6d9bf9ffc869424075f108bfad57ca5ed17a0310 Git commit d17b3cfa756af28278d61ba9756aed418ec57b38 by Maik Qualmann. Committed on 11/09/2015 at 16:52. Pushed by mqualmann into branch 'master'. implements full-screen slideshow from the current image in the Light Table preview M +6 -0 utilities/lighttable/lighttableview.cpp M +3 -0 utilities/lighttable/lighttableview.h M +52 -56 utilities/lighttable/lighttablewindow.cpp M +7 -3 utilities/lighttable/lighttablewindow.h M +0 -2 utilities/lighttable/lighttablewindow_p.h http://commits.kde.org/digikam/d17b3cfa756af28278d61ba9756aed418ec57b38 Git commit 3b5960fbd54723aee595f8abe437957c91001dd3 by Gilles Caulier. Committed on 15/09/2015 at 10:49. Pushed by cgilles into branch 'frameworks'. backport commit #d17b3cfa756af28278d61ba9756aed418ec57b38 from git/master to frameworks branch M +8 -1 utilities/lighttable/lighttableview.cpp M +3 -0 utilities/lighttable/lighttableview.h M +52 -56 utilities/lighttable/lighttablewindow.cpp M +7 -3 utilities/lighttable/lighttablewindow.h M +0 -2 utilities/lighttable/lighttablewindow_p.h http://commits.kde.org/digikam/3b5960fbd54723aee595f8abe437957c91001dd3 *** Bug 361689 has been marked as a duplicate of this bug. *** Is there a way to have that effect (full screen overlay) when double clicking on an image (instead of clicking on the icon)? No. The icon is dedicated for FS mode. Double click on thumbnail open Editor or preview mode. The user can see the main interface visible in opposite of FS mode. Gilles Caulier It seems to me the current behaviour is once again that fullscreen button starts a slideshow instead of simply showing the photo. Is this intended? I personally think showing the image in fullscreen should be totally different from showing a slideshow. Those are totally different actions. I totally agree with Luca Carlon. Since one of the last updates a slide show is started automatically, which is something different. It's actually quite annoying to always have to set it to pause. It would be nice to at least have a konfiguration option to decide whether a slide show should be started or not. In any case however, opening an image to fullscreen should be a simple and immediate action, it cannot require to click on a small button, then going down and clicking on another small button within a few seconds. I propose again this concept: my personal opinion is that double clicking the image in preview should open the image to fullscreen. This is what most environments have been doing for years: double clicking "opens" something. Esc button "closes" something. My opinion is that it would feel more natural: double clicking from thumbnail opens the image in preview, double clicking shows fullscreen from preview, ESC goes back to preview, ESC again goes back to thumbnails. Slideshow can remain an overlay button. I totally agree with Luca Carlon. Digikam needs a simple and immediate action to open images in fullscreen (no slideshow!). His concept using a double click and ESC is simple and easy and it is used in many other media players/viewers. If this is a regression, I suggest to report it separately. It's not a regression. It have been always implemented like this since a while... Gilles Caulier What is described in comment #44 seems fine, but please please please maintain a way to easily get from thumbnail to full screen. It's what I use all the time and having to go to preview first and from there to full screen and the same way back would be a pain. Where is the problem. Here where in digiKAm you can switch in Full Screen by a keyboard shortcut : SHIFT+CTRL+F. Same shortcut to go out from full screen. This will work in previe mode or in thumbnail mode, or in map mode or in table view mode... etc. Bref, it's universal. We don't need to bloat the icn view with more overlay. Please let's the thumbnail visible : the photo contents is more important than a way to change the GUI. Learn the shortcuts, and you will see that digiKam has already a lots of features for you. That all... I think he means a full screen mode only for the image. And yes, I am the solution with the slideshow also not happy. I plan to implement a full screen mode to replace slideshow as full screen view. - no overlays over the image view - scroll wheel of the mouse changes the images - for 1:1 viewing or zooming, remember the last position to compare the images by scrolling before and back. Maik Maik, my original patch was doing this. It might be lost on the way. Except I left the overlays on the bottom and the was no zoom I think. The rest was more or less like this. More than 3 years later, again no way to simply put the image to fullscreen without having a complete slideshow right? This is marked as resolved, shouldn't it be opened again? |