D11245: infopanel: Add choice of date display formats
Mark Gaiser
noreply at phabricator.kde.org
Wed Mar 14 22:18:18 GMT 2018
markg requested changes to this revision.
markg added a comment.
This revision now requires changes to proceed.
In D11245#224414 <https://phabricator.kde.org/D11245#224414>, @ngraham wrote:
> I think having it in the context menu makes enough sense; the toggle for the preview is already there, so there's already precedent for having the Information Panel's UI be controlled via context menu. +1.
@broulik and I seem to think that it should be in the settings, not under the RMB.
To be fair, i'm trusting your judgement so i'm fine either way, but i'm "leaning" towards in the settings as well.
INLINE COMMENTS
> informationpanelcontent.cpp:119
> m_metaDataWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Minimum);
> -
> + m_metaDataWidget->setDateFormat((Baloo::DateFormats) InformationPanelSettings::dateFormat());
> +
No c-style cast.
> informationpanelcontent.cpp:198
> if (m_metaDataWidget) {
> + m_metaDataWidget->setDateFormat((Baloo::DateFormats) InformationPanelSettings::dateFormat());
> m_metaDataWidget->show();
No c-style cast.
> informationpanelcontent.cpp:290
> + dateformatAction->setCheckable(true);
> + dateformatAction->setChecked(InformationPanelSettings::dateFormat() == (int) Baloo::DateFormats::ShortFormat);
> popup.addSeparator();
No c-style cast.
> informationpanelcontent.cpp:308-311
> + auto dateFormat = static_cast<int>(
> + isChecked
> + ? Baloo::DateFormats::ShortFormat
> + : Baloo::DateFormats::LongFormat);
Err, auto for int.. No, just int! Don't misuse auto.
Also, put this on one line. Nobody formats a cast like that and it imho makes it less readable.
REPOSITORY
R318 Dolphin
REVISION DETAIL
https://phabricator.kde.org/D11245
To: michaelh, #dolphin, markg
Cc: broulik, markg, ngraham, spoorun, navarromorales, isidorov, firef, andrebarros, alexeymin, genaxxx, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180314/baea54ea/attachment.htm>
More information about the kfm-devel
mailing list