Show dialogs on button clicked (not pressed)

Review Request #123546 - Created April 28, 2015 and submitted

Information
David Rosca
kaccounts-integration
master
Reviewers
telepathy
mklapetek

Connect to QPushButton::clicked signal instead of QPushButton::pressed.

This makes it consistent with other buttons, eg. the button action is activated only when releasing the mouse inside the button rect, not immediately on mouse press.
It is also needed to change QCommandLinkButton to QPushButton, where it changes the text alignment from left-aligned to center-aligned. I don't know how to replicate the look with
QPushButton, but we should not use QCommandLinkButton as it is activated on press (and there is no way to easily change it).

Buttons are now activated on mouse release.

David Edmundson
David Rosca
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted with commit 47734ffd90bdd40350379ffecf7f055fb0eec9a8 by David Rosca to branch master.
Martin Klapetek

but we should not use QCommandLinkButton as it is activated on press (and there is no way to easily change it).

QCommandLinkButton derives from QPushButton, it has exactly the same signals including clicked(); there's really no reason to change that. I'll be changing it back to QCommandLinkButton.

  1. That's correct, sorry. When I tested it, for some reason I incorrectly thought the clicked signal is emitted on button press.
    But there actually is now a reason to use QPushButton instead of QCommandLinkButton: QCommandLinkButton don't have vertically aligned icon and text in Breeze style ( http://wstaw.org/m/2015/05/26/qcommandlinkbutton-breeze.png this is with this commit reverted).
    It should be fixed in Breeze, but still...

  2. Yes, that's a bug in different product and should be fixed there, we shouldn't really be working around it, especially if we have an (relatively easy) way of fixing the cause of the bug. Plus, QPushButton cannot horizontally align things, which imo is a bigger (visual) issue ;)

  3. For record keeping: https://bugs.kde.org/show_bug.cgi?id=348257

  4. There is also issue with Fusion (and win9x) style where QCommandLinkButton is always flat (setFlat(false) has no effect). Not to mention that QCommandLinkButton is "Vista style command link button".
    I think it would be better to use custom painting to achieve button contents aligned to left than to use QCommandLinkButton (which is really rarely used widget, lxr search shows only this code actually using it).

  5. Personally I don't mind the flat look in Fusion. But fair enough, if you can come up with custom control based on QPushButton, I'll be happy to review that. But until then, I'll revert this (besides the clicked part).

    For the record, we've used this button in all of kde-telepathy's history and there wasn't a single report about them being wrong in any way.

Loading...