broulik added inline comments.
INLINE COMMENTS
> kdeplatformsystemtrayicon.cpp:27
> #include <QApplication>
> +#include <QVariant>
>
Already included in the header file
> kdeplatformsystemtrayicon.cpp:33
> : QPlatformMenu()
> - , m_enabled(true)
> - , m_visible(true)
> - , m_separatorsCollapsible(true)
> + , m_enabled(QVariant::Bool)
> + , m_visible(QVariant::Bool)
Is this explicit initialization neccessary?
> kdeplatformsystemtrayicon.cpp:188
> + if (!m_enabled.isNull()) {
> + m_menu->setEnabled(m_enabled.value<bool>());
> + }
Prefer the specific methods, if exist, `toBool()`
> kdeplatformsystemtrayicon.h:60
> QIcon m_icon;
> - bool m_enabled;
> - bool m_visible;
> - bool m_separatorsCollapsible;
> + QVariant m_enabled;
> + QVariant m_visible;
(This looks like a job for `std::optional`?)
REPOSITORY
R135 Integration for Qt applications in Plasma
REVISION DETAIL
https://phabricator.kde.org/D25223
To: kmaterka, #plasma, #frameworks, broulik
Cc: cgiboudeaux, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
