D20525: [InformationPanel] Use the new inline configuration mode

Elvis Angelaccio noreply at phabricator.kde.org
Sat Apr 20 15:25:52 BST 2019


elvisangelaccio added inline comments.

INLINE COMMENTS

> informationpanelcontent.cpp:113-114
> +    m_configureLabel = new QLabel(i18nc("@label::textbox",
> +                                          "Select which data should "
> +                                          "be shown:"), this);
> +    m_configureLabel->setWordWrap(true);

Why split the i18n sentence into two lines? It looks weird imho.

> informationpanelcontent.cpp:118
> +
> +    m_configureButtons = new QDialogButtonBox(QDialogButtonBox::Save|QDialogButtonBox::Cancel);
> +    m_configureButtons->setVisible(false);

Coding style: missing space before/after `|`

> informationpanelcontent.cpp:122-124
> +                m_configureButtons->setVisible(false);
> +                m_configureLabel->setVisible(false);
> +                emit configurationFinished();

These 3 lines could go in a dedicated function which we could call from both lambdas, to avoid code duplication. Or it could even be a private slot connected to `QDialogButtonBox::clicked`.

REPOSITORY
  R318 Dolphin

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

To: bruns, #dolphin, #baloo, #frameworks, ngraham, astippich, #vdg, elvisangelaccio
Cc: elvisangelaccio, meven, kfm-devel, alexde, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190420/4d263738/attachment.html>


More information about the Kde-frameworks-devel mailing list