Created attachment 120394 [details] screenshot STEPS TO REPRODUCE 1. open discover 2. click Plasma addons > Plasma themes in the side bar 3. click on any plasma theme with two or more screenshots to open its description page OBSERVED RESULT As we can see in the attached screenshot, there is empty space between the thumbnail of the screenshots. Discover opens a screenshot when you click on the empty space. EXPECTED RESULT no empty space between the thumbnail of the screenshots. SOFTWARE/OS VERSIONS Operating System: KDE neon Unstable Edition KDE Plasma Version: 5.16.80 KDE Frameworks Version: 5.59.0 Qt Version: 5.12.0
Feels like an issue with the image we get back from the OCS API. Dan, does that make sense? Or is this a Discover issue?
Yes, this would be because the thumbnail in OCS is kind of small... i feel like this is something we already fixed once... There isn't really a mid-size thumbnail concept in OCS, just a classic quite-small-thumbnail (for e.g. listviews and the like) and a full-size image, so if we need anything of a certain large-ish size, like in Discover's application page, we're supposed to use the big preview image. So, that being the theory i seem to remember that we did this change in Discover already, but it also seems like we, in fact, did not. I think i can see why we didn't - the semantics are somewhat awkward, where the term "thumbnail" is used for the small image url used in the overview, while the reality is that that isn't really a thumbnail at all, but rather a mid-sized preview. However, it also is a bit odd, as this is a new issue, and unless my memory is completely wonky here, we /used/ to get mid-sized preview images as thumbnails from OCS, so it's quite possible that something weird has happened with the default crop for thumbnails on the store.
Thanks for the info! If the thumbnails we get are tiny, can we just reduce the padding in between them in Discover, then? Or is there any way of requesting full-size images that Discover itself can scale down to make its own thumbnails? This is what it does with other backends, where there generally isn't a separate thumbnail image.
Yeah, that's probably the best way to go about it... Yes, simply use the PreviewBig options at all times, instead of trying to also use PreviewSmall (the code is in... KNSResource, i'm thinking it might make sense to just return the screenshots qlist for both screenshot types rather than the small preview one for the small option...). Though yes, it'd probably be good to also fix the screenshots component and make it behave a little better when the images it's showing are a bit small (which i'm sure would still happen).
Sounds good, let's do it!
Git commit cf8606eb89f70011edc170a5a2ec5a699933cfcf by Aleix Pol. Committed on 14/06/2019 at 23:50. Pushed by apol into branch 'Plasma/5.16'. kns (mostly): Don't show weird padding if the thumbnails are too small Limit the width of each thumbnail by the original width. M +1 -1 discover/qml/ApplicationScreenshots.qml https://commits.kde.org/discover/cf8606eb89f70011edc170a5a2ec5a699933cfcf