enhance KRecentFilesAction with a ui action to remove entries

David Faure faure at kde.org
Fri Mar 13 00:23:12 GMT 2009


On Wednesday 11 March 2009, Christoph Pfister wrote:
> 2009/3/7, Christoph Pfister <christophpfister at gmail.com>:
> > Patch and screenshot attached - comments welcome (don't know how the
> > k-c-d development model works, but I don't mind you / me commiting it
> > if it's ok).
> 
> v2 of the patch:
> - renamed *enable to *visible
> - replaced "Remove all entries" by "Clear list"
> - only show the clear action if the list isn't empty
> - internal: don't delete / reconstruct the special actions, instead
> use setVisible
> - internal: do not put the special actions into selectableActionGroup,
> as the code assumes that those actions are recent actions (e.g. for
> maxItems calculation and in saveEntries)
> 
> Please treat questions like "clear action visible by default" or
> "clear action in general" seperately (the clear action is still
> disabled by default in my patch); if you reach consensus about that it
> can be easily done as a follow-up commit.


+void KRecentFilesAction::setClearActionVisible(bool visible)
+{
+    Q_D(KRecentFilesAction);
+    d->clearActionVisible = visible;
+    visible = visible && !selectableActionGroup()->actions().isEmpty();
+    d->clearSeparator->setVisible(visible);
+    d->clearAction->setVisible(visible);
+}

Do not reuse the boolean "visible", define a new one. It will help readability.

-  virtual void clear();

I'm not sure whether reordering virtuals preserves BC... the comment says
it comes from a base class so I assume so, but, hmm, for extra safety
I would leave it where it is and just add "public Q_SLOTS:" before
(and whatever is needed after like "public:").


Personally I don't see why this feature needs to be optional (getter/setter/property... all useless IMHO). I see no reason why the
user should be prevented from clearing that list. I'm not obeying your
request to treat this issue separately, because if we agree that
it should be on by default and everywhere, then the setting isn't
necessary anymore. But this isn't a strong opinion, just a preference
for consistency over pleasing individual application developers.

-- 
David Faure, faure at kde.org, sponsored by Qt Software @ Nokia to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).




More information about the kde-core-devel mailing list