Should be all fixed now.

I'll put this on resolved for now, we can create new tasks for the rest :)

There's still quite a bit we can do, but yes, you can now browse an opds catalog and download the open-access items.

With https://invent.kde.org/frameworks/knewstuff/-/merge_requests/95 being merged, is this fully implemented now?

This was tracked in https://bugs.kde.org/show_bug.cgi?id=415606 and has already been fixed. Please use https://bugs.kde.org for bug reporting :)

I also want to mention that it has not very much in common with the rest of KNewStuff, other than that it is on the same repo. Maybe it should be split up in a small, separate lib.

In D19338#676682, @kossebau wrote:

In D19338#676680, @asturmlechner wrote:

Hm, why was this new variable KDE_INSTALL_KNSRCDIR not put into KDEInstallDirs?

On the other hand it makes sense that module-specific variables are provided by the CMake config file of the respective module. Which allows projects to use the module without having to use KDEInstallDirs, but e.g. GnuInstallDirs.

In D19338#676680, @asturmlechner wrote:

Hm, why was this new variable KDE_INSTALL_KNSRCDIR not put into KDEInstallDirs?

Hm, why was this new variable KDE_INSTALL_KNSRCDIR not put into KDEInstallDirs?

No problem, always happy to help 😃

Thanks for making me realise that it doesn't have to be quite so elaborate, @alex ;)

As @alex suggests, just use qlist::contains, it is supposed to be
reasonably cheap, so... yup, trust the framework! ;)

Address comment by @alex

KDE Unstable at this point is 5.17.80
KDE Frameworks is version 5.67.0
Qt Version: 5.13.2

Because I am unsure if you can interrupt a loop in a different function, I tried to figure out what KIO does. KIO seems to have a tiny jungle of jobs calling subjobs telling workers to suspend, and eventually the whole thing ends up at ConnectionBackend, which seems (TCP) Socket based. (With a local and remote modes, much like our download jobs).

That does indeed look good, and not a huge departure from what we have already, which i'm sure will make people feel quite at home in the new version :) I do wonder if we'd want to, at some point, make it look closer to Discover, but certainly for now this seems a much more sensible direction.

Will be good see how it looks in dark theme, especially dark pictures.

Looks great, though I would put the search field on top. It's a lot like the new GridView KCMs; maybe we can even use that template for it. The current Colors KCM looks a lot like that.

@ngraham: Fo sure, I will add my post from the forum here for better reference.

In D20693#455251, @leinir wrote:

i'm afraid the "it works" is an illusion brought on by looking at content which doesn't show the issue... The main problem here is that we're dealing with user-generated content, and we can't assume that just because an image has a certain size, that space is actually filled with image data :/

In D20693#454874, @ngraham wrote:

To show that a thumbnail clickable, switching to the pointing hand cursor when hovering over a thumbnail could work.

However I notice that the actual list delegates in the browse view seem to add frames and shadows to the thumbnails there, and they look okay. The frame's proportions even perfectly match the aspect ratio of the thumbnail:

Why doesn't any of that work here?

To show that a thumbnail is clickable, switching to the pointing hand cursor when hovering over a thumbnail could work.

In D20693#454799, @sitter wrote:

In D20693#454750, @leinir wrote:

So yes, in principle i'd certainly like there to be some kind of background or outline to suggest clickability, but the current state (and any other generic rectangular background we might come up with, higher resolution or not) would yield the same suboptimal result...

QGraphicsDropShadowEffect via QWidget::setGraphicsEffect may work for that?

Another approach would likely be to write a custom blur implementation since we fiddle with qpainter anyway, TBH I think finding a way to use the effect is likely the wise use of time though.

In D20693#454750, @leinir wrote:

So yes, in principle i'd certainly like there to be some kind of background or outline to suggest clickability, but the current state (and any other generic rectangular background we might come up with, higher resolution or not) would yield the same suboptimal result...

In D20693#454783, @sitter wrote:

LGTM on a technical level. On a visual level also +1 because I hate that drop shadow with a fierce passion.

@leinir lxr says a similar thumb is also used for some of the KAboutPerson stuff in kxmlgui. it may be prudent to also remove the thumb there, I expect it looks equally dated.

LGTM on a technical level. On a visual level also +1 because I hate that drop shadow with a fierce passion.

In D20693#454733, @ndavis wrote:

+1 for the change to the large thumbnail, but I think the smaller thumbnails need something to show that they can be clicked.

+1 for the change to the large thumbnail, but I think the smaller thumbnails need something to show that they can be clicked.

Adding VDG because this is a visual change. To the visual commenters: This is intended as a first step, removing the old drop-shadow method (see also the related bug for a more severe version of that drop-shadow being shown very incorrectly). If we decide we do want a drop shadow, then that will want to be done separately. Note also that as we cannot guarantee the image will not have any translucent areas (which is the cause of the highly nasty look in the bug report, rather than just the basic nastiness shown in the screenshots here), we will need to do something considerably more clever than just putting a 9-slice drop image around the image (for example), which might well be a fair bit more expensive computationally than we'd probably want.

Before patch:

In D20693#454173, @ngraham wrote:

This patch doesn't apply:

 INFO  Base commit is not in local repository; trying to fetch.
Created and checked out branch arcpatch-D20693.
Checking patch src/ui/imagepreviewwidget_p.h...
Checking patch src/ui/imagepreviewwidget.cpp...
Checking patch data/thumb_frame.png...
error: the patch applies to 'data/thumb_frame.png' (afaf432793864e1fb3f1fc27aa1d53689f2243b5), which does not match the current contents.
error: data/thumb_frame.png: patch does not apply
Checking patch data/CMakeLists.txt...
Applied patch src/ui/imagepreviewwidget_p.h cleanly.
Applied patch src/ui/imagepreviewwidget.cpp cleanly.
Applied patch data/CMakeLists.txt cleanly.

 Patch Failed! 
Usage Exception: Unable to apply patch!

Also it would be helpful if you used arc for your patches, since then you can see the context here in the web UI: https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist_to_post_patches

Attempt to use arcanist, hopefully with less data loss this time