[WIP] Bring KNewStuffQuick to feature parity with KNewStuff(Widgets)
Needs ReviewPublic

Authored by leinir on Jun 10 2019, 1:11 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

As the title suggests, this is very much WIP. Feel free to test things out, and indeed point out issues, but as with all WIPs it's entirely possible those are already known. (this includes things like "there is no test plan", "there are no reviewers", or "this is just a list of individual commit messages and not a good summary" ;) )

Basic KNewStuffQuick dialog for testing purposes

Actually return the number of comments, not the rating

Change the install overwrite question to YesNo

It was previously a very uninformative question, so rewording
it to a YesNo question, with more verbosity, seems reasonable,
given this is potentially destructive.

Add a QuestionListener for KNewStuffQuick

This was promised in the documentation already, and missing,
which is a bit silly. Let's just implement that ;)

Add the Question enums to its metaobject

Expose method to get the row of a specific EntryInternal

Add the QtQuick QuestionAsker component

A component used to forward questions from KNewStuff's
engine to the UI, similar to how the KNewStuff::WidgetQuestionListener
class works.

Slightly modified GridDelegate, to allow for a tile style view

Based on the KCM GridDelegate

Add the Ratings component from Discover

Should this perhaps be in a more centrally usable location? Seems
like something we'd probably want to be using in more locations,
for a more consistent look when ratings are involved...

Add a model exposing categories metadata to QtQuick

A model to care for the comments for a single EntryInternal

This is a step along the way to add comment support to KNewStuff

Actually build the CommentsModel

Implement support for Attica's comments support in KNSCore::Provider

Add some getters to KNSCore::Engine (name, search fields, and comments)

Expose the various new functionality in KNSCore::Engine

This includes the name, categories, and filter and sort options

Add Comments, adoption and full installation support to Quick ItemsModel

Actually build and expose our various new classes in KNewStuffQuick

Add a screenshots display component, based heavily on Discover's

Very basic (for now) comment delegate

A Kirigami.ScrollablePage for showing an entry's comments

A Kirigami.OverlaySheet for picking a specific DownloadItem to install

Add a component for viewing an Entry's details (KCM.SimpleKCM based)

Add a DownloadDialog equivalent to KNewStuffQuick

This page is partly based on design work found on:
https://phabricator.kde.org/D20693#455565
and partly on discussions with the VDG. The BigPreview delegate
specifically was envisioned by Andy Betts

Add the new components to the qmldir

Diff Detail

Repository
R304 KNewStuff
Branch
knsquick-feature-parity-with-kns (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13874
Build 13892: arc lint + arc unit
leinir created this revision.Jun 10 2019, 1:11 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJun 10 2019, 1:11 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
leinir requested review of this revision.Jun 10 2019, 1:11 PM
leinir edited the summary of this revision. (Show Details)Jun 10 2019, 1:15 PM

How does one test this? Does it replace the existing one?

How does one test this?

By running the khotnewstuff-dialog application and passing a knsrc file to it (though i would point to the first line in the summary for the moment before you do too much testing ;) ).

Does it replace the existing one?

It is supposed to end up at feature parity with the Widget based tools, but not entirely replace that (perhaps as much as deprecate). It is not quite intended as a drop-in replacement, but rather i'm building it so it is more flexible than the old system, which will allow people to embed it more tightly in their applications, or have it almost entirely disconnected, as they wish.

Thanks! I think we should definitely aim to replace the old QWidgets-based one. Otherwise we're signing up for an increased maintenance burden with no real advantages IMO. Consider that we already have the GHNS dialog as well as Discover's UI, and each one has subtle bugs fixed in the other one. Adding a third frontend for the same data would make this even worse. Ideally over time we move to just having one desktop-based frontend (this one maybe?) with multiple ways to access it.

Thanks! I think we should definitely aim to replace the old QWidgets-based one. Otherwise we're signing up for an increased maintenance burden with no real advantages IMO. Consider that we already have the GHNS dialog as well as Discover's UI, and each one has subtle bugs fixed in the other one. Adding a third frontend for the same data would make this even worse. Ideally over time we move to just having one desktop-based frontend (this one maybe?) with multiple ways to access it.

Yeah - obviously we can't rip out the old one yet, but we can mark it deprecated and pending removal in KF6 :)

Could the new one be API compatible so we wouldn't need to wait for KF6? Also it's always nice to avoid breaking API if you don't have to.

Could the new one be API compatible so we wouldn't need to wait for KF6? Also it's always nice to avoid breaking API if you don't have to.

Not entirely sure we can do that, the old KNewStuff widgets are quite tightly bound to the widget world... It might be possible with a touch of heavy duty header wrangling (keeping it abi compatible would be a fair bit of work, though not impossible). At the very least, though, i'd like to perhaps do that as a sort of... second step after these components have been created. In short: Doable, but quite possibly more work than it'd be worth (when compared to straight up deprecation-and-porting-guide).

leinir updated this revision to Diff 60021.Tue, Jun 18, 1:35 PM

Highlights: NewStuffButton, basic Kiosk support (needs more clever and user-facing mention of why the thing they just tried to do didn't happen/do anything), various fixing, cleanup, and sanity work.

  • Fix logic for canFetchMore (initial state was a touch funny)
  • Explicitly request the listed categories for searching
  • Fix the emptiness issue in the proper place, and properly
  • Don't need to set this, now it's a proper model
  • Slightly more roundabout, but saner way for the API to handle the engine
  • Missed two files in previous commit
  • Allow opening any random knsrc file on the user's system
  • Expose a changedEntries property on the KNSQuick Engine
  • Import the proper NewStuff version
  • Add the initial version of a KNSQuick Button
  • Add a NewStuff.Button to the khns-dialog test app
  • Add allowedByKiosk in the Quick engine, and expose it
  • Add allowedByKiosk, and mark a few things invokable
  • Add a NewStuffDialog component (and content)
  • Minor doc change
  • Add the NewStuffDialog and NewStuffDialogContent components
  • Much simpler NewStuffButton (and use the Engine's kiosk info)
pino added a subscriber: pino.Tue, Jun 18, 2:21 PM
pino added inline comments.
src/core/engine.h
471

BIC change, you cannot add virtual functions in a public class

src/core/provider.h
150

BIC change, you cannot add virtual functions in a public class

leinir marked 2 inline comments as done.Wed, Jun 19, 9:17 AM
leinir added inline comments.
src/core/engine.h
471

That shouldn't be virtual anyway, not sure why i did that...

src/core/provider.h
150

That's a fair bit more annoying... Oh well, signal-based workaround it is, i guess (not like we have a lot of providers anyway ;) )

leinir updated this revision to Diff 60052.Wed, Jun 19, 12:38 PM
leinir marked 2 inline comments as done.

Address pino's comments about BIC (and some documentation)

  • Don't mark functions virtual which should not be virtual
  • Apply the signal/slot virt func workaround for improved BIC
  • Add a bit more documentation (and some @since)
leinir updated this revision to Diff 60181.Fri, Jun 21, 8:34 AM
  • A pile of style cleaning
  • Implement the adoptItem function in KNSQuick::ItemsModel
leinir updated this revision to Diff 60204.Fri, Jun 21, 10:08 AM
  • Sort the default sizing on the Dialog to be more pleasant
  • Rejig the Dialog's button implementation, so it's now got shortcuts
  • Set the default column width of the test dialog

QList<KNSCore::Comment*> should be QList<shared_ptr<KNSCore::Comment>>, don't leave ownership to consumers.

leinir updated this revision to Diff 60214.Fri, Jun 21, 11:09 AM

Address anthonyfieroni's comment about manual memory management in the comments fetching system

  • Switch to a shared_ptr list for comments (less manual memory mgmnt)
leinir updated this revision to Diff 60689.Wed, Jun 26, 1:22 PM
  • Fill the width in the dialog's sidebar with Button
  • Use the knsrc file's Name attr for title (already translated)
  • Indent the comments by their depth (this is imperfect, but...)
leinir updated this revision to Diff 60729.Thu, Jun 27, 1:48 PM
  • Work on the comments delegate's look some
leinir updated this revision to Diff 60781.Fri, Jun 28, 1:57 PM

Bunch of work on the comments (especially avatar stuff). In progress, of course, but the concept is there

  • Add an Author component to the QtQuick bits
  • Add Providers functionality for fetching person data
  • Add avatar and description fields to the KNSCore::Author class
  • Clean up the depth count (and less crashing)
  • Add author details to the QtQuick ItemsModel
  • We're version 1.1
  • Remember to remember the namespace
  • The cards listview is great... but i need indentation :/
  • Use the new magic Author component
leinir updated this revision to Diff 60920.Mon, Jul 1, 12:31 PM
  • Uncrash the author hash (don't use const QString references as key...)
leinir updated this revision to Diff 61065.Wed, Jul 3, 12:58 PM
  • Add profile page to the author classes
  • Add a spacer item to the CommentsPage listview
  • A touch of visual cleaning in the comment delegate
  • Type-safe comparisons are good
  • Throttle the fetch calls just a touch, no reason to be spammy
leinir updated this revision to Diff 61331.Mon, Jul 8, 11:24 AM

Thanks kbroulik!

  • allowedByKiosk should be a CONSTANT property, as it's... constant
  • Postpone engine initialisation until Button is clicked
leinir updated this revision to Diff 61340.Mon, Jul 8, 1:33 PM
  • Add the user ID to the author class
  • Expose the entry author id through the qtquick item model
  • Rename the entryAuthor property to entryAuthorId, as that's what it is
  • Add a bit more info to the author
  • Add a copy ctor to Author
  • Make sure we check the right things, and format the author correctly
leinir updated this revision to Diff 61341.Mon, Jul 8, 1:51 PM
  • Put the NewStuff.Author component to use on the details page as well
leinir updated this revision to Diff 61500.Wed, Jul 10, 10:48 AM
  • Add a downloaditemssheet to the entry details page
leinir updated this revision to Diff 61638.Fri, Jul 12, 11:02 AM
  • A small spacer item for the entry details page
  • Documentation++
  • Show a passive notification in the dialog when messages are posted