[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: [PATCH] Fix buttons accessibility in KMenuEdit (Bug296682)
From:       Albert Astals Cid <aacid () kde ! org>
Date:       2012-12-08 21:57:02
Message-ID: 1945927.A2sRfQWjaG () xps
[Download RAW message or body]

El Dijous, 22 de novembre de 2012, a les 20:24:08, Albert Astals Cid va 
escriure:
> El Dijous, 22 de novembre de 2012, a les 03:24:47, Christoph Feck va 
escriure:
> > On Wednesday 21 November 2012 19:05:18 Julien wrote:
> > > Hi,
> > > 
> > > This is a patch for the bug
> > > https://bugs.kde.org/show_bug.cgi?id=296682
> > > 
> > > It prevents to make buttons as inactive after having deleted or
> > > cutted an item.
> > > 
> > > Thus, this bug does not only concern the delete button after having
> > > deleted an item, but also the following buttons : cut and copy,
> > > after having deleted OR cutted an item.
> > > 
> > > It's my first patch submit :), I wanted to post it in the
> > > ReviewBoard, but it seems that the Kmenuedit group does not exist.
> > > 
> > > Thanks
> > 
> > Thanks for the investigation. The patch is wrong, though.
> > 
> > In TreeView::itemSelected() the actions are explictely re-enabled. Due
> > to a Qt3 -> Qt4 porting regression, the connection (see line 210), is
> > wrong. To keep track of newly selected items, you should connect to
> > "itemSelectionChanged", not "currentItemChanged" signal.
> > 
> > Also, the "setCurrentItem" calls are probably wrong, too. In Qt4,
> > current != selected, so to reselect the next item after cut/delete,
> > you have to use the overload that accepts the
> > QItemSelectionModel::SelectionFlags command.
> 
> Have you actually tried the patched software? Becuase, yes you are right in
> all that you say, but i can't find any problem while using it, so I wouldn't
> say the patch is wrong.

I've commited Julien's code to master, feel free to make it better.

Cheers,
  Albert

> 
> Cheers,
>   Albert
> 
> > Christoph Feck (kdepepo)
> > KDE Quality Team
> > 
> > >> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to
> > >> unsubscribe
> > >> <<
> >> 
> >> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe
> >> <<

>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
[prev in list] [next in list] [prev in thread] [next in thread]