mart added a comment.
I would like this class to be splitted in Templates/controls, so different
styles could make it look slightly sifferent (or anyways easier for the app
developer to make one that looks just slightly different)
the root component should be a Control, with image/icon/initials in
contentItem and background collor/border in background
the one in templates would not have a background the one in controls would
instantiate the one in templates and give it one.
INLINE COMMENTS
> avatar.cpp:34
> +
> +const QList<QColor> c_colors = {
> + QColor("#e93a9a"),
would be nice to be somewhat theme-able, maybe with the controls/templates split
> Avatar.qml:15
> + */
> +Rectangle {
> + id: avatarRoot
should be a Control, with some stuff moved into background and some into
contentItem
there should be one in templates with no background and one here that
instatiates the templates one and adds the background
> Avatar.qml:28
> + }
> + enum InitialsMode {
> + UseInitials,
this is not really necessary, if one doesn't wasnt the initials, doesn't set a
name.
> Avatar.qml:52
> + * Whether the button should always show the image; show the image if one
> is
> + * available and show initials when it is not; or always show initials.
> + */
enum values should be written and explained here
> Avatar.qml:109
> + layer.enabled: true
> + layer.effect: OpacityMask {
> + maskSource: mask
we now have a much more efficient component for that: ShadowedImage, which
should be used instead
> Avatar.qml:115
> + }
> + Rectangle {
> + color: "transparent"
this looks like something that should be themeable
> kirigamiplugin.cpp:259
>
> + qmlRegisterSingletonType<AvatarPrivate>("org.kde.kirigami.private", 2,
> 13, "AvatarPrivate", [] (QQmlEngine*, QJSEngine*) -> QObject* { return new
> AvatarPrivate; });
> + qmlRegisterType(componentUrl(QStringLiteral("Avatar.qml")), uri, 2, 13,
> "Avatar");
I don't like this, as even tough is called "private" is really public api as
there isn't really any way to make a c++ type not accessible.
nothing in that c++ class seems really to be necessary to be c++
REPOSITORY
R169 Kirigami
REVISION DETAIL
https://phabricator.kde.org/D29694
To: cblack, #kirigami, #vdg
Cc: mart, ratijastk, ngraham, filipf, plasma-devel, fbampaloukas, GB_2, domson,
dkardarakos, apol, ahiemstra, davidedmundson
