Differential D28154 Diff 78135 kcms/users/package/contents/ui/UserDetailsPage.qml

Changeset View

Standalone View

View Options

kcms/users/package/contents/ui/UserDetailsPage.qml

  • This file was added.
1​/*
2​ Copyright 2019 Nicolas Fella <nicolas.fella@gmx.de>
3​ Copyright 2020 Carson Black <uhhadd@gmail.com>
4​
5​ This library is free software; you can redistribute it and/or
6​ modify it under the terms of the GNU Lesser General Public
7​ License as published by the Free Software Foundation; either
8​ version 2.1 of the License, or (at your option) version 3, or any
9​ later version accepted by the membership of KDE e.V. (or its
10​ successor approved by the membership of KDE e.V.), which shall
11​ act as a proxy defined in Section 6 of version 3 of the license.
12​
13​ This library is distributed in the hope that it will be useful,
14​ but WITHOUT ANY WARRANTY; without even the implied warranty of
15​ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
16​ Lesser General Public License for more details.
17​
18​ You should have received a copy of the GNU Lesser General Public
19​ License along with this library. If not, see <http://www.gnu.org/licenses/>.
20​*/
21​
22​import QtQuick 2.6
23​import QtQuick.Layouts 1.3
24​import QtQuick.Controls 2.5
25​import QtQuick.Dialogs 1.3
26​
ngrahamUnsubmitted
Done

...as QQC2

ngraham: ...`as QQC2`
27​import org.kde.kcm 1.2
28​import org.kde.kirigami 2.8 as Kirigami
29​import org.kde.users 1.0
30​
31​SimpleKCM {
32​ id: connectionEditorPage
33​
34​ title: user.realName
35​
36​ property variant user
37​
broulikUnsubmitted
Done

Use property var, or even property QtObject since User is a QObject

broulik: Use `property var`, or even `property QtObject` since `User` is a `QObject`
38​ Connections {
39​ target: kcm
40​
41​ onApply: {
42​ user.realName = realNametextField.text
43​ user.email = emailTextField.text
44​ user.name = userNameField.text
45​ user.administrator = (usertypeBox.currentIndex == 1)
46​ user.apply()
47​ }
48​ }
49​
50​ Menu {
51​ id: imagePickerMenu
broulikUnsubmitted
Done

No user picture gallery anymore?

broulik: No user picture gallery anymore?
52​
53​ MenuItem {
54​ text: i18n("Change account picture")
55​ icon.name: "folder-open"
56​ onClicked: fileDialog.open()
57​ }
58​
59​ MenuItem {
60​ text: i18n("Delete account picture")
61​ icon.name: "edit-delete"
62​ onClicked: {
63​ kcm.needsSave = true
64​ user.face = ""
65​ }
66​ }
67​ }
68​
69​ FileDialog {
70​ id: fileDialog
broulikUnsubmitted
Done

Start in pictures folder?
I think you can set that by doing

folder: shortcuts.pictures
broulik: Start in pictures folder? I think you can set that by doing ``` folder: shortcuts.pictures ```
71​ title: "Choose a picture"
72​ folder: shortcuts.pictures
73​ onAccepted: {
74​ kcm.needsSave = true
75​ user.face = fileDialog.fileUrl
76​ }
77​ }
78​
79​ ColumnLayout {
80​
81​ RowLayout {
82​ Layout.alignment: Qt.AlignHCenter
83​ ToolButton {
broulikUnsubmitted
Done

i18n

broulik: `i18n`
84​ property int size: 6 * Kirigami.Units.gridUnit
85​
86​ icon.name: user.faceValid ? "" : "user-identity"
87​ icon.width: size
88​ icon.height: size
89​
90​ Image {
broulikUnsubmitted
Done

Set a sourceSize to keep it from loading a full size image when user choses an absurdly large image

broulik: Set a `sourceSize` to keep it from loading a full size image when user choses an absurdly large…
ngrahamUnsubmitted
Done

This should be cropped into a circle, like we do in Kickoff and the login/lock/logout screens. You can probably just lift the code straight from Kickoff.

ngraham: This should be cropped into a circle, like we do in Kickoff and the login/lock/logout screens.
91​ source: user.face
92​ visible: user.faceValid
93​ sourceSize: Qt.size(parent.size, parent.size)
94​ anchors {
95​ fill: parent
96​ margins: Kirigami.Units.smallSpacing
97​ }
broulikUnsubmitted
Not Done

Is this a RoundButton bug that it doesn't indicate keyboard focus when I tab to it?

broulik: Is this a `RoundButton` bug that it doesn't indicate keyboard focus when I tab to it?
98​ }
broulikUnsubmitted
Done

Pfp?

broulik: Pfp?
cblackAuthorUnsubmitted
Not Done

profile picture

cblack: profile picture
99​
100​ Layout.alignment: Qt.AlignHCenter
broulikUnsubmitted
Done

readonly property

broulik: `readonly property`
101​ Layout.maximumHeight: size + Kirigami.smallSpacing
102​ Layout.minimumHeight: size + Kirigami.smallSpacing
ngrahamUnsubmitted
Done

Nice! How about an outline around the circle like we do in Kickoff?

ngraham: Nice! How about an outline around the circle like we do in Kickoff?
103​ Layout.maximumWidth: size + Kirigami.smallSpacing
104​ Layout.minimumWidth: size + Kirigami.smallSpacing
105​
106​ onClicked: imagePickerMenu.popup()
107​ }
108​ TextField {
109​ id: realNametextField
110​ focus: true
111​ text: user.realName
broulikUnsubmitted
Done

Add context i18nc("Example name", "John Doe")

broulik: Add context `i18nc("Example name", "John Doe")`
112​ placeholderText: i18nc("Example name", "John Doe")
broulikUnsubmitted
Done

I *think* one needs to multiple that with Screen.devicePixelRatio

broulik: I *think* one needs to multiple that with `Screen.devicePixelRatio`
113​
broulikUnsubmitted
Done

Use onEditingFinished

broulik: Use `onEditingFinished`
114​ onEditingFinished: {
115​ kcm.needsSave = true
116​ }
117​ }
118​ }
119​ Kirigami.FormLayout {
120​ TextField {
121​ id: userNameField
122​ focus: true
123​ text: user.name
124​ Kirigami.FormData.label: i18n("Username:")
125​
126​ onTextChanged: {
127​ kcm.needsSave = true
128​ }
129​ }
130​
131​ ComboBox {
132​ id: usertypeBox
133​
broulikUnsubmitted
Done

I'd prefer if you did

textRole: "label"
model: [
    {type: "standard", label: i18n("Standard")},
    {type: "admin", label: i18n("Administrator")}
]

and then in the place where you check admin check for type rather than index === 1

broulik: I'd prefer if you did ``` textRole: "label" model: [ {type: "standard", label: i18n…
134​ model: [
135​ i18n("Standard"),
136​ i18n("Administrator"),
137​ ]
138​
139​ Kirigami.FormData.label: i18n("Account type:")
140​
141​ currentIndex: user.administrator ? 1 : 0
broulikUnsubmitted
Done

Looks unused

broulik: Looks unused
142​
143​ onActivated: {
144​ kcm.needsSave = true
145​ }
146​ }
147​
148​ TextField {
149​ id: emailTextField
150​ focus: true
151​ text: user.email
152​ placeholderText: i18nc("Example email address", "john.doe@kde.org")
153​ Kirigami.FormData.label: i18n("Email address:")
154​
broulikUnsubmitted
Done

Add i18nc

broulik: Add `i18nc`
155​ onTextChanged: {
156​ kcm.needsSave = true
157​ }
158​ }
159​
160​ Button {
161​ text: i18n("Change Password")
162​ onClicked: {
163​ changePassword.account = user
164​ changePassword.openAndClear()
165​ }
166​ }
167​
168​ Item {
169​ Layout.preferredHeight: deleteUser.height
170​ }
171​
172​ Button {
173​ id: deleteUser
ngrahamUnsubmitted
Done

Does it make sense to be able to delete the currently-logged-in user, or the only user on the system?

ngraham: Does it make sense to be able to delete the currently-logged-in user, or the only user on the…
174​
175​ text: i18n("Delete User")
176​ icon.name: "delete"
177​ onClicked: UserController.deleteUser(user.uid)
178​ }
ngrahamUnsubmitted
Done

Doesn't work for me; clicking the button has no effect.

ngraham: Doesn't work for me; clicking the button has no effect.
179​ }
180​ }
181​
182​ ChangePassword { id: changePassword; account: user }
183​}
ngrahamUnsubmitted
Done

Might be nice to give this sheet a title, so that there's something in its header area besides a close button.

ngraham: Might be nice to give this sheet a title, so that there's something in its header area besides…
ngrahamUnsubmitted
Done

This sheet ends up really narrow, so there are only ever two columns of icons visible, even with a very wide window. It doesn't look great IMO. Wider would be better.

ngraham: This sheet ends up really narrow, so there are only ever two columns of icons visible, even…
ngrahamUnsubmitted
Done

This button needs text so people can figure out what it does.

ngraham: This button needs text so people can figure out what it does.
ngrahamUnsubmitted
Done

This doesn't seem to work. When I change the avatar using the sheet, the image on the details page gets reset to the default, rather than reflecting my choice.

ngraham: This doesn't seem to work. When I change the avatar using the sheet, the image on the details…
ngrahamUnsubmitted
Done

Despite this, they're still jaggy for me. Maybe play with the mipmapping settings?

ngraham: Despite this, they're still jaggy for me. Maybe play with the mipmapping settings?
ngrahamUnsubmitted
Done

Still not fixed for me.

ngraham: Still not fixed for me. {F8249187, size=full}
ngrahamUnsubmitted
Done

this isn't working anymore for me

ngraham: this isn't working anymore for me
ngrahamUnsubmitted
Done

This still doesn't work right. Selecting a custom image doesn't make it get applied when you click on the apply button, and I still can't clear avatars.

ngraham: This still doesn't work right. Selecting a custom image doesn't make it get applied when you…
ngrahamUnsubmitted
Done

This no longer asks if you want to delete that user's home directory, so it's always preserved.

ngraham: This no longer asks if you want to delete that user's home directory, so it's always preserved.
ngrahamUnsubmitted
Done

I think "Choose from File..." would be more appropriate (and don't forget the ellipses, since it opens a file dialog)

ngraham: I think "Choose from File..." would be more appropriate (and don't forget the ellipses, since…
ngrahamUnsubmitted
Done

This menu doesn't close when you click outside of it; you need to hit the escape key.

ngraham: This menu doesn't close when you click outside of it; you need to hit the escape key.
ngrahamUnsubmitted
Done

Give it an icon: edit-delete

ngraham: Give it an icon: `edit-delete`
ngrahamUnsubmitted
Done

Give it an icon: document-multiple

ngraham: Give it an icon: `document-multiple`
ngrahamUnsubmitted
Done

edit-delete

ngraham: `edit-delete`
broulikUnsubmitted
Done

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;
}
broulik: Can you add some way to make Escape close the sheet but not the entire KCM, when run standalone…
ngrahamUnsubmitted
Done

FWIW KCMs opened through KRunner are now opened in System Settings so KCMShell is something we're going to have to deal with much less. :)

ngraham: FWIW KCMs opened through KRunner are now opened in System Settings so KCMShell is something…
broulikUnsubmitted
Done

Yes, but it's still something to consider. Right now Esc does nothing at all, and neither do arrow keys. This thing needs some FocusScope treatment

broulik: Yes, but it's still something to consider. Right now Esc does nothing at all, and neither do…
broulikUnsubmitted
Done

Is this RowLayout needed?

broulik: Is this `RowLayout` needed?
cblackAuthorUnsubmitted
Not Done

it's a convenience for the default alignment of a layout (Qt.AlignVCenter | Qt.AlignLeft)

cblack: it's a convenience for the default alignment of a layout (`Qt.AlignVCenter | Qt.AlignLeft`)
broulikUnsubmitted
Not Done

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.

broulik: Can we not hardcode the list of avatars, please. Also, you probably want to use a proper…
broulikUnsubmitted
Done

A tooltip and Accessible.name with the name of the avatar would be nice

broulik: A tooltip and `Accessible.name` with the name of the avatar would be nice
broulikUnsubmitted
Not Done

Is this ColumnLayout needed?

broulik: Is this `ColumnLayout` needed?
broulikUnsubmitted
Not Done

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

broulik: What's all of this? I don't see anything in the UI. Anyway, we probably want to make all of…
broulikUnsubmitted
Not Done

Not sure if clever or mad :) Can we not just generate a colored rectangle on C++ side?

broulik: Not sure if clever or mad :) Can we not just generate a colored rectangle on C++ side?