rkflx accepted this revision.
rkflx added a comment.

  In https://phabricator.kde.org/D7828#160998, @emateli wrote:
  
  > If this turns out to be okay, I'll edit the summary and title later
  
  
  Playing around with Dolphin's usage of `createKMessageBox`, I made the 
following observations:
  
  - Focus: Before on checkbox, now on first button.
  - Default: Before on some random button (changing with app restart), now 
consistently on first button.
  - `setFocus` to a different button was broken and is still broken.
  - `setDefault` to a different button was broken and is still broken.
  
  This is a great improvement and already fits Dolphin's case. As this does not 
regress any other usage (remember, this was broken already), I would be in 
favour of committing this now (after adapting the commit message).
  
  @aacid As you "requested changes", would you be okay with that?
  
  That said, a followup patch would be marvelous (add autotests, fix `setFocus` 
and `setDefault`).
  
  In https://phabricator.kde.org/D7828#161102, @elvisangelaccio wrote:
  
  > Does this mean that apps should always pass a parent? (if that's the case, 
we should mention it in the KMessageBox documentation)
  
  
  If at all, apps should //never// pass a parent, because it gets overwritten 
anyway. @emateli is just moving the code for this around, not changing it. But 
as the docs already state "This function will take care of connecting to it.", 
adding something about a parent will probably only cause more confusion.

REVISION DETAIL
  https://phabricator.kde.org/D7828

To: emateli, #frameworks, ngraham, aacid, #vdg, rkflx
Cc: elvisangelaccio, rkflx, abetts, subdiff, ngraham, aacid, #frameworks

Reply via email to