broulik added a comment.
Pretty cool
INLINE COMMENTS
> UserDetailsPage.qml:36
> +
> + property variant user
> + property bool overrideImage: false
Use `property var`, or even `property QtObject` since `User` is a `QObject`
> UserDetailsPage.qml:82
> + id: fileDialog
> + title: "Choose a picture"
> + folder: shortcuts.pictures
`i18n`
> UserDetailsPage.qml:96
> + Layout.alignment: Qt.AlignHCenter
> + QQC2.RoundButton {
> + id: userPfp
Is this a `RoundButton` bug that it doesn't indicate keyboard focus when I tab
to it?
> UserDetailsPage.qml:97
> + QQC2.RoundButton {
> + id: userPfp
> +
Pfp?
> UserDetailsPage.qml:99
> +
> + property int size: 6 * Kirigami.Units.gridUnit
> +
`readonly property`
> UserDetailsPage.qml:111
> + visible: usersDetailPage.user.faceValid ||
> usersDetailPage.overrideImage
> + sourceSize: Qt.size(parent.size, parent.size)
> + cache: false
I *think* one needs to multiple that with `Screen.devicePixelRatio`
> UserDetailsPage.qml:217
> +
> + Kirigami.OverlaySheet {
> + id: picturesSheet
Can you add some way to make Escape close the sheet but not the entire KCM,
when run standalone through kcmshell? Hopefully the following is sufficient,
otherwise you'd have to mess with `FocusScope` and the like:
Keys.onEscapePressed: {
close();
event.accepted = true;
}
> UserDetailsPage.qml:219
> + id: picturesSheet
> + header: RowLayout {
> + Kirigami.Heading {
Is this `RowLayout` needed?
> UserDetailsPage.qml:287
> + model: [
> + "Artist Konqi.png",
> + "Bookworm Konqi.png",
Can we not hardcode the list of avatars, please.
Also, you probably want to use a proper `GridView` for all of this, otherwise
you end up creating every single delegate immediately on opening.
> UserDetailsPage.qml:342
> +
> + QQC2.Button {
> + Layout.preferredHeight: Kirigami.Units.gridUnit * 6
A tooltip and `Accessible.name` with the name of the avatar would be nice
> UserDetailsPage.qml:346
> +
> + ColumnLayout {
> + anchors.centerIn: parent
Is this `ColumnLayout` needed?
> UserDetailsPage.qml:361
> +
> + Repeater {
> + model: [
What's all of this? I don't see anything in the UI.
Anyway, we probably want to make all of this into a proper `QAbstractListModel`
> UserDetailsPage.qml:399
> + onClicked: {
> + colourRectangle.grabToImage(function(result)
> {
> + picturesSheet.close()
Not sure if clever or mad :) Can we not just generate a colored rectangle on
C++ side?
> usermodel.cpp:112
> +{
> + if (!checkIndex(index))
> + {
I just learned that you need to pass
`QAbstractItemModel::CheckIndesOptions::IndexIsValid` otherwise it only checks
for blatant out of bounds indices but not invalid "Null" ones :/
REPOSITORY
R119 Plasma Desktop
BRANCH
arcpatch-D28154
REVISION DETAIL
https://phabricator.kde.org/D28154
To: cblack, #plasma, #vdg, ngraham
Cc: ltoscano, mart, yurchor, iasensio, meven, crossi, The-Feren-OS-Dev,
davidedmundson, broulik, filipf, ngraham, nicolasfella, zzag, plasma-devel,
Orage, LeGast00n, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot,
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol,
ahiemstra
