D21971: Replace ToolAction by ToggleActionMenu

Simone Gaiarin noreply at phabricator.kde.org
Sun Jan 19 18:55:28 GMT 2020


simgunz added a comment.


  You need to merge master into this review or rebase because there is a merge conflict. I suggested you the modifications that were introduced in master (`qAsConst`) and few other changes.

INLINE COMMENTS

> toggleactionmenu.cpp:93
> +{
> +    for ( QAction * a : menu->actions() )
> +    {

Change to `qAsConst( menu->actions() )`.
Change `a` to `action` for clarity.

> toggleactionmenu.cpp:101
> +        {
> +            QAction * b = checkedAction( a->menu() );
> +            if ( b )

Something more explicit than `b`? ` Like `checked` or `actionChecked`.

> toggleactionmenu.cpp:113
> +{
> +    for ( QPointer< QToolButton > button : qAsConst( m_buttons ) )
> +    {

I would change to `QToolButton * button`.

> toggleactionmenu.cpp:123
> +
> +            if (delayed())
> +            {

Spaces around delayed() for uniformity.

> toggleactionmenu.cpp:125
> +            {
> +                button->setPopupMode(QToolButton::DelayedPopup);
> +            }

Spaces around `QToolButton::DelayedPopup` for uniformity (also in statements below).

REPOSITORY
  R223 Okular

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

To: davidhurka, simgunz
Cc: aacid, ngraham, simgunz, okular-devel, johnzh, andisa, siddharthmanthan, maguirre, fbampaloukas, joaonetto, kezik, tfella, darcyshen
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20200119/6a3aafd6/attachment.html>


More information about the Okular-devel mailing list