Cyberbeat wrote:
> Hi,
Hi too :)
> I made modifications to the classic-kmenu-applet:
>
> I send a svn-diff for an overview of the changes to existing files. And an
> archive with all modified/new files. I will be happy if I get
> feedback/suggestions. This is my first qt/kde-programming-experience :)
I would say, that it looks pretty good and even more great if it's your first
qt/kde-programming-experience! Thanks for that. So, some comments;
+ QLabel *showRecentLabel = new QLabel(i18n("Recent applications:"), p);
+ l->addWidget(showRecentLabel, 2, 0);
+ d->showRecentCheckBox = new QCheckBox(i18n("Show on top of standard
menu"),p);
+ d->showRecentCheckBox->setChecked(d->showRecentApplications);
+ showRecentLabel->setBuddy(d->showRecentCheckBox);
+ l->addWidget(d->showRecentCheckBox, 2, 1);
+
+ QLabel *recentLabel = new QLabel(i18n("Recent applications:"), p);
+ l->addWidget(recentLabel, 3, 0);
+ d->recentApplicationsSpinBox = new QSpinBox(p);
+ d->recentApplicationsSpinBox->setMaximum(10);
+ d->recentApplicationsSpinBox->setMinimum(0);
+
d->recentApplicationsSpinBox->setValue(Kickoff::RecentApplications::self()->maximum());
+ recentLabel->setBuddy(d->recentApplicationsSpinBox);
+ l->addWidget(d->recentApplicationsSpinBox, 3, 1);
Two times a "Recent applications:" label? I would suggest to remove the first
label and change the text of the checkbox to something like "Show recent
applications" or so. Maybe the second label should then say "Number of recent
application items" (my english sucks but the point here is, to indicate that
the spin changes the number of items what is useful to know if e.g. a
screenreader does read that label).
Do you think it would be a good idea to introduce logic to disable or even
hide those both options if some other view like e.g. "logout" was choosen
where those options don't make sense?
- <signal name="cleanRecentDocumentsAndDocuments"/>
+ <signal name="clearRecentDocumentsAndApplications"/>
That's a bit of a problem cause it changes the dbus-API. That would mean, that
e.g. scripts or applets written against <4.3 connecting with that signal
won't work any longer with 4.3 what is evil (specially since it's a
runtime-error probably hard to discover). But then I guess it's safe to
assume that noone outside of kickoff uses that signal yet. Anyway, I would
suggest to backport that fix to the yet unreleased 4.2 branch before 4.2 is
out and others may start to use that signal.
- Q_ASSERT(maximum > 0);
+ Q_ASSERT(maximum >= 0);
uh? Does it really make sense to allow to set 0 recent items? I would suggest
to use d->recentApplicationsSpinBox->setMinimum(1);
+ while (privateSelf->serviceQueue.count() > privateSelf->maxServices) {
+ QString removeId = privateSelf->serviceQueue.takeFirst();
+ kDebug() << "More than max services added. Removing" <<
removeId << "from queue.";
+ privateSelf->serviceInfo.remove(removeId);
+ emit
applicationRemoved(KService::serviceByStorageId(removeId));
+ }
Sorry, probably a stupid question, but does that also affect the recentmodel
used in kickoff (so, not the classic menu)? If yes, then that's a problem
cause that option can only be changed in the classic menu but is used at both
of them. If not, then forget this comment :)
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
