D29447: Fix showing updates when the option is selected
This revision was automatically updated to reflect the committed changes. Closed by commit R304:25d391c6f36b: Fix showing updates when the option is selected (authored by leinir). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29447?vs=82021&id=82064 REVISION DETAIL https://phabricator.kde.org/D29447 AFFECTED FILES src/core/itemsmodel.cpp src/qtquick/qml/Page.qml src/qtquick/quickitemsmodel.cpp To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D29447: Fix showing updates when the option is selected
alex added a comment. No problem, always happy to help 😃 REPOSITORY R304 KNewStuff BRANCH fix-show-only-updates (branched from master) REVISION DETAIL https://phabricator.kde.org/D29447 To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D29447: Fix showing updates when the option is selected
leinir marked 2 inline comments as done. leinir added a comment. Thanks for making me realise that it doesn't have to be quite so elaborate, @alex ;) REPOSITORY R304 KNewStuff BRANCH fix-show-only-updates (branched from master) REVISION DETAIL https://phabricator.kde.org/D29447 To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D29447: Fix showing updates when the option is selected
leinir updated this revision to Diff 82021. leinir added a comment. As @alex suggests, just use qlist::contains, it is supposed to be reasonably cheap, so... yup, trust the framework! ;) - Just use qlist::contains REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29447?vs=82003&id=82021 BRANCH fix-show-only-updates (branched from master) REVISION DETAIL https://phabricator.kde.org/D29447 AFFECTED FILES src/core/itemsmodel.cpp src/qtquick/qml/Page.qml src/qtquick/quickitemsmodel.cpp To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D29447: Fix showing updates when the option is selected
leinir added inline comments.
INLINE COMMENTS
> alex wrote in itemsmodel.cpp:71
> On second thought why not just use:
> `bool duplicate = m_entries.contains(entry);`
Hmm... i do wonder slight of the cost of that, but also... much simpler code,
so... basically can just go
if (!m_entries.contains(entry)) {
below, rather than this more elaborate version.
> alex wrote in itemsmodel.cpp:71
> Use qAsConst(m_entries) and space before & not after :-)
Quite, thank you :)
REPOSITORY
R304 KNewStuff
BRANCH
fix-show-only-updates (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D29447
To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks
Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack,
jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns,
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol,
ahiemstra, mart
D29447: Fix showing updates when the option is selected
alex added inline comments.
INLINE COMMENTS
> itemsmodel.cpp:71
> +bool duplicate{false};
> +for (const EntryInternal &existingEntry : qAsConst(m_entries)) {
> +if (existingEntry == entry) {
On second thought why not just use:
`bool duplicate = m_entries.contains(entry);`
REPOSITORY
R304 KNewStuff
BRANCH
fix-show-only-updates (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D29447
To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks
Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack,
jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns,
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol,
ahiemstra, mart
D29447: Fix showing updates when the option is selected
ngraham accepted this revision. This revision is now accepted and ready to land. REPOSITORY R304 KNewStuff BRANCH fix-show-only-updates (branched from master) REVISION DETAIL https://phabricator.kde.org/D29447 To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D29447: Fix showing updates when the option is selected
leinir updated this revision to Diff 82003. leinir added a comment. Address comment by @alex - Fix style (and consty things) REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29447?vs=81993&id=82003 BRANCH fix-show-only-updates (branched from master) REVISION DETAIL https://phabricator.kde.org/D29447 AFFECTED FILES src/core/itemsmodel.cpp src/qtquick/qml/Page.qml src/qtquick/quickitemsmodel.cpp To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D29447: Fix showing updates when the option is selected
alex added inline comments.
INLINE COMMENTS
> itemsmodel.cpp:71
> +bool duplicate{false};
> +for (const EntryInternal& existingEntry : m_entries) {
> +if (existingEntry == entry) {
Use qAsConst(m_entries) and space before & not after :-)
REPOSITORY
R304 KNewStuff
REVISION DETAIL
https://phabricator.kde.org/D29447
To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks
Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack,
jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns,
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol,
ahiemstra, mart
D29447: Fix showing updates when the option is selected
leinir added reviewers: KNewStuff, Plasma, bugseforuns, ngraham, Frameworks. leinir added projects: Plasma, KNewStuff. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D29447 To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks Cc: kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D29447: Fix showing updates when the option is selected
leinir created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. leinir requested review of this revision. REVISION SUMMARY This patch primarily fixes the issue that when picking the "Updateable Only" option on NewStuff.Page, we would be shown all the entries anyway. It also fixes duplicated entries on the "Installed Only" option, and further adds a little placeholder for when the view is expectedly empty. - Use the right connections for update entry display - While potentially expensive, only add entries to the model once - Use Kirigami.PlaceholderMessage to show we have no entries when we don't BUG:416762 TEST PLAN Without this patch, behaviour is as shown in https://bugs.kde.org/show_bug.cgi?id=416762 With this patch, the behaviour is as expected (shows only updateable entries with that option selected, only shows installed entries once with that option, and shows a useful message when the list is empty and expected to be empty) REPOSITORY R304 KNewStuff BRANCH fix-show-only-updates (branched from master) REVISION DETAIL https://phabricator.kde.org/D29447 AFFECTED FILES src/core/itemsmodel.cpp src/qtquick/qml/Page.qml src/qtquick/quickitemsmodel.cpp To: leinir Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
