D27540: KCModule: Indicate when a setting has been changed from the default or previous value

[David Edmundson]

davidedmundson added a comment.


  What do we want to happen for released code that gets a bugfix update?

INLINE COMMENTS

> kconfigdialogmanager.cpp:609
> +    const auto defaultValue = [item] {
> +        item->swapDefault();
> +        const auto value = item->property();

Why not item->readDefault()?

> kconfigdialogmanager.cpp:625
> +            q->setProperty(widget, defaultValue);
> +            updateWidgetIndicator(configId, widget);
> +            emit q->widgetModified();

won't it do it itself when the property changes?

> settingsstatusindicator.cpp:75
> +    setFocusPolicy(Qt::NoFocus);
> +    show();
> +}

> This is equivalent to calling showFullScreen(), showMaximized(), or setVisible(true), depending on the platform's default behavior for the window flags.

For X and wayland it's setVisible(true)

but we shouldn't count on it.

> settingsstatusindicator.cpp:175
> +    const auto leftToRight = m_trackedWidget->isLeftToRight();
> +    auto x = leftToRight ? m_trackedWidget->pos().x() + m_trackedWidget->width()
> +                         : m_trackedWidget->pos().x() - width();

unused?

> settingsstatusindicator.cpp:184
> +        const auto re = QRegularExpression("^kcfg_");
> +        const auto children = m_trackedWidget->parentWidget()->findChildren<QWidget*>(re, Qt::FindDirectChildrenOnly);
> +        const auto xValues = [=] {

Can we be sure the tracked widget always has a parent widget?

If someone doesn't use layouts a widget might not have a parent.

> settingsstatusindicator.cpp:192
> +                               const auto localX = leftToRight ? widgetExpectedWidth(w) : -width();
> +                               return w->pos().x() + localX;
> +                           });

that's not true for the RTL case where the widget is expected to resize.

It would be   w->pos().x() + w->width() - widgetExpectedWidth(w)

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis, broulik
Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200331/435ea1f4/attachment-0001.html>