kossebau added inline comments.

INLINE COMMENTS

> kbusyindicatorwidget.h:40
> + * is more specific.
> + */
> +class KWIDGETSADDONS_EXPORT KBusyIndicatorWidget : public QWidget

Any chance of getting some samples how this class is supposed to be used?

Sounds one should show & hide the complete widget when needed? How to best 
integrate in one's layout? As overlay?

BTW, the KDE HIG does not mention such a spinner. So the purpose from a KDE 
developer following the HIG raises a question with me wearing my naive hat :)
https://hig.kde.org/components/assistance/progress.html

> kbusyindicatorwidget.h:57-58
> +private:
> +    class Private;
> +    Private *const d;
> +};

You can make this a non-nested class by implcitly forward declaring here:

  class KBusyIndicatorWidgetPrivate *const d;

This allows not needing the Q_DECL_HIDDEN for the symbols of the then no longe 
nested private class.

Even more fancy:

  QScopedPointer<class KBusyIndicatorWidgetPrivate> const d;

No longer the need to delete the d explicitely :)
Though perhaps not familiar in look of cod too many.

> kbusyindicatorwidgettest.cpp:31
> +    w.setBaseSize(128, 128);
> +    w.show();
> +

Could this test be somehow extended to cover repeated show & hide?

REPOSITORY
  R236 KWidgetsAddons

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

To: sitter, cfeck
Cc: kossebau, broulik, kde-frameworks-devel, apol, LeGast00n, michaelh, 
ngraham, bruns

Reply via email to