D28154: Add users KCM

filipf added a comment. In D28154#672453 , @ngraham wrote: > Now that D29394 has landed, you can add those images here! . REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.or

D28154: Add users KCM

cblack closed this revision. REPOSITORY R119 Plasma Desktop 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-deve

D28154: Add users KCM

cblack updated this revision to Diff 83095. cblack added a comment. Rename to kcm_users REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=83079&id=83095 BRANCH arcpatch-D28154_1 REVISION DETAIL https://phabricator.kde.org/D28154 AFFECTED

D28154: Add users KCM

meven added a comment. So next step would be to shadow user_manager ? A naive ripgrep gave : $ rg user_manager .. ../kxmlgui/src/kbugreport.cpp 352:m_process->start(QStringLiteral("kcmshell5"), QStringList() << QStringLiteral("user_manager")); ../systemsettings

D28154: Add users KCM

cblack added inline comments. INLINE COMMENTS > broulik wrote in UserDetailsPage.qml:97 > Pfp? profile picture > broulik wrote in UserDetailsPage.qml:219 > Is this `RowLayout` needed? it's a convenience for the default alignment of a layout (`Qt.AlignVCenter | Qt.AlignLeft`) REPOSITORY R11

D28154: Add users KCM

ngraham added a comment. Great, that works! BTW I think you can change the KCM name to `kcm_users` as you originally wanted. Sorry for saying otherwise. REPOSITORY R119 Plasma Desktop BRANCH arcpatch-D28154_1 REVISION DETAIL https://phabricator.kde.org/D28154 To: cblack, #plasma

D28154: Add users KCM

cblack updated this revision to Diff 83079. cblack added a comment. Focus the stackSwitcher when opening the sheet REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=83074&id=83079 BRANCH arcpatch-D28154_1 REVISION DETAIL https://phabricat

D28154: Add users KCM

ngraham added a comment. The avatar sheet still does not close with Esc. :/ REPOSITORY R119 Plasma Desktop BRANCH arcpatch-D28154_1 REVISION DETAIL https://phabricator.kde.org/D28154 To: cblack, #plasma, #vdg, ngraham Cc: ltoscano, mart, yurchor, iasensio, meven, crossi, The-Feren-OS-

D28154: Add users KCM

cblack updated this revision to Diff 83074. cblack marked 17 inline comments as done. cblack added a comment. Address code concerns; add escape keyboard gesture REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82214&id=83074 BRANCH arcpatch

D28154: Add users KCM

ngraham added a comment. Now that D29394 has landed, you can add those images here! REPOSITORY R119 Plasma Desktop BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154 To: cblack, #plasma, #vdg, ngraham Cc: ltoscano, mart, yur

D28154: Add users KCM

broulik added inline comments. INLINE COMMENTS > ngraham wrote in UserDetailsPage.qml:217 > 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. :) Yes, but it's still something to consider. Right now Esc does

D28154: Add users KCM

ngraham added inline comments. INLINE COMMENTS > broulik wrote in UserDetailsPage.qml:217 > 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`

D28154: Add users KCM

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 > +

D28154: Add users KCM

ngraham added a comment. In D28154#668362 , @ltoscano wrote: > Technically kcm should be called kcm_. So far the Plasma_version part has been ignored, but at least kcm_ should be there. > > I'm not sure about the lack of the button. I

D28154: Add users KCM

ltoscano added a comment. Technically kcm should be called kcm_. So far the Plasma_version part has been ignored, but at least kcm_ should be there. I'm not sure about the lack of the button. REPOSITORY R119 Plasma Desktop BRANCH arcpatch-D28154 REVISION DETAIL https://phabricato

D28154: Add users KCM

yurchor added a subscriber: ltoscano. yurchor added a comment. Please do not get me wrong (I'm fine with the new designs and updating the code and this new KCM is very good), but it would be good to hear some words from @ltoscano on the catalog (and KCM) naming. It is also evident that a

D28154: Add users KCM

ngraham added a comment. I would like to get this into 5.19, which would mean moving the code back into the user-manager repo, or removing it as a dependency for Plasma 5.19 by requesting an exception to the repo freeze. REPOSITORY R119 Plasma Desktop BRANCH arcpatch-D28154 REVISION DE

D28154: Add users KCM

davidedmundson added a comment. From my POV. +1 I still need a reply on: "Which release?" REPOSITORY R119 Plasma Desktop BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154 To: cblack, #plasma, #vdg, ngraham Cc: mart, yurchor, iasensio, meven, crossi, T

D28154: Add users KCM

cblack updated this revision to Diff 82214. cblack marked 4 inline comments as done. cblack added a comment. Parent temporary file to the application REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82212&id=82214 BRANCH arcpatch-D28154 RE

D28154: Add users KCM

cblack updated this revision to Diff 82212. cblack added a comment. realize you forgot to change the name of the argument in the body as well REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82211&id=82212 BRANCH arcpatch-D28154 REVISION D

D28154: Add users KCM

davidedmundson added inline comments. INLINE COMMENTS > cblack wrote in kcm.cpp:121 > that looks like it could possibly run into race conditions, as the applyjob > is executed asynchronously and IIRC is able to still be running when > everything else, including this, has destructed and removed

D28154: Add users KCM

cblack updated this revision to Diff 82211. cblack added a comment. Refine method signature and add a comment for less ambiguity REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82210&id=82211 BRANCH arcpatch-D28154 REVISION DETAIL https

D28154: Add users KCM

cblack added inline comments. INLINE COMMENTS > davidedmundson wrote in kcm.cpp:121 > Normally the app creating a temp file would clean it up. > > A quick fix would be > file = new QTemporaryFile(this); > > It's sort of leaking the object, but on exit it'll tidy everything up. that looks like

D28154: Add users KCM

nicolasfella added inline comments. INLINE COMMENTS > davidedmundson wrote in kcm.cpp:97 > OH! > > Initial-ise > > not initialise as in "to prepare" > > Makes sense :) Then it should have a different name, like "getInitials", else the next person will be just as confused REPOSITORY R119 P

D28154: Add users KCM

davidedmundson added inline comments. INLINE COMMENTS > cblack wrote in kcm.cpp:121 > whoever is normally cleaning up tempfiles Normally the app creating a temp file would clean it up. A quick fix would be file = new QTemporaryFile(this); It's sort of leaking the object, but on exit it'll tidy

D28154: Add users KCM

cblack updated this revision to Diff 82210. cblack marked an inline comment as done. cblack added a comment. Parent the dynamically created user to the model REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82209&id=82210 BRANCH arcpatch-D2

D28154: Add users KCM

davidedmundson added inline comments. INLINE COMMENTS > cblack wrote in kcm.cpp:97 > taking the initials of a user's real name OH! Initial-ise not initialise as in "to prepare" Makes sense :) > usermodel.cpp:37 > +connect(m_dbusInterface, &OrgFreedesktopAccountsInterface::UserAdded, > t

D28154: Add users KCM

cblack updated this revision to Diff 82209. cblack marked 4 inline comments as done. cblack added a comment. Address some of the code feedback from d_ed REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82158&id=82209 BRANCH arcpatch-D28154

D28154: Add users KCM

cblack marked 5 inline comments as done. cblack added inline comments. INLINE COMMENTS > davidedmundson wrote in kcm.cpp:94 > what's this for? used as a hook for QML to know when the KCM needs to apply > davidedmundson wrote in kcm.cpp:97 > This is an odd method name. > > I don't really unders

D28154: Add users KCM

davidedmundson added a comment. Which release? We need to remove user-manager. We're technically in a repo freeze, and this would involve changing (removing) a repo a breakage of that. Generally this looks quite neat and tidy. A big simplification +++ INLINE COMMENTS

D28154: Add users KCM

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Great. From my perspective this is ready to go! I'd like a #plasma person to review too, and for @mart or another #plasma

D28154: Add users KCM

cblack updated this revision to Diff 82158. cblack marked 9 inline comments as done. cblack added a comment. Address code and icon feedback REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82150&id=82158 BRANCH arcpatch-D28154 REVISION DET

D28154: Add users KCM

ngraham added a subscriber: mart. ngraham added a comment. Nice, that fixes the last of the avatar issues that I can find. After you add icons to the items in the keep/delete menu and make it close it with a click outside the menu, I think I'll be done reviewing the UI--other than the mu

D28154: Add users KCM

cblack updated this revision to Diff 82150. cblack added a comment. Fix faceValid checking REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82137&id=82150 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154 AFFECTE

D28154: Add users KCM

ngraham added a comment. Changing a user's avatar and clicking Reset instead of Apply makes the avatar display revert to the default avatar (which I remember you telling me isn't actually possible), not the previous one. This seems inaccurate because the avatar does in fact remain at the las

D28154: Add users KCM

cblack updated this revision to Diff 82137. cblack added a comment. begone, kule REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82135&id=82137 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154 AFFECTED FILES

D28154: Add users KCM

cblack updated this revision to Diff 82135. cblack added a comment. Make sure arc diff does what it should REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82048&id=82135 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org

D28154: Add users KCM

ngraham added inline comments. INLINE COMMENTS > ChangePassword.qml:24 > +import QtQuick.Layouts 1.3 > +import QtQuick.Controls 2.5 > + ...`as QQC2` > CreateUser.qml:24 > +import QtQuick.Layouts 1.3 > +import QtQuick.Controls 2.5 > + ...`as QQC2` > UserDetailsPage.qml:25 > +import QtGraphical

D28154: Add users KCM

cblack added inline comments. INLINE COMMENTS > ngraham wrote in main.qml:54 > Could this be a BasicListItem with a title and subtitle, or is that not > possible because you can't render the avatar+circle into an image source to > feed it? Not possible, AFAIK. At least not as long as grabbing

D28154: Add users KCM

cblack updated this revision to Diff 82048. cblack added a comment. Address code issue and add reset functionality REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82046&id=82048 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator

D28154: Add users KCM

ngraham added a comment. Great. We're getting pretty close to the UI being finished IMO. I'll start on the QML code review soon. In the meantime I found a few more little behavioral issues: 1. The Reset button correctly activates when the KCM is in a dirty state, but doesn't seem to act

D28154: Add users KCM

cblack updated this revision to Diff 82046. cblack marked an inline comment as done. cblack added a comment. Fix item alignment REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82025&id=82046 BRANCH arcpatch-D28154 REVISION DETAIL https:

D28154: Add users KCM

ngraham added inline comments. INLINE COMMENTS > main.qml:141 > +QQC2.Button { > +Layout.alignment: Qt.AlignLeft > + This doesn't work because there's an item on the left of it that fills the width. Remove that item, or this alignment property (and whichever one you choose,

D28154: Add users KCM

cblack updated this revision to Diff 82025. cblack marked 4 inline comments as done. cblack added a comment. Address feedback REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82020&id=82025 BRANCH arcpatch-D28154 REVISION DETAIL https://

D28154: Add users KCM

ngraham added inline comments. INLINE COMMENTS > main.qml:141 > +QQC2.Button { > +Layout.alignment: Qt.AlignRight > + Align left to match the emerging standard for other {nad Add new thing} buttons > user_manager.desktop:9 > +X-KDE-Weight=40 > +X-KDE-System-Settings-Parent-C

D28154: Add users KCM

cblack updated this revision to Diff 82020. cblack added a comment. Fix permission denied bouncing REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82016&id=82020 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154

D28154: Add users KCM

ngraham added a comment. I found a few more little things: 1. This is not a big deal, but could the grid of avatars be horizontally centered in its sheet? F8287538: Screenshot_20200505_132938.png 2. Text and icons in the Change Password sheet a

D28154: Add users KCM

cblack updated this revision to Diff 82016. cblack added a comment. Add Messages.sh REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82015&id=82016 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154 AFFECTED FILES

D28154: Add users KCM

cblack updated this revision to Diff 82015. cblack added a comment. Add Messages.sh REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82012&id=82015 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154 AFFECTED FILES

D28154: Add users KCM

ngraham added a comment. Love the section headers! REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D28154 To: cblack, #plasma, #vdg, ngraham Cc: yurchor, iasensio, meven, crossi, The-Feren-OS-Dev, davidedmundson, broulik, filipf, ngraham, nicolasfella, zzag, p

D28154: Add users KCM

yurchor added a comment. Is it possible to add Messages.sh to make this KCM translatable? #! /usr/bin/env bash $XGETTEXT `find . -name "*.cpp" -o -name "*.qml"` -o $podir/user_manager.pot Thanks in advance for your work. REPOSITORY R119 Plasma Desktop REVISION DETAIL https

D28154: Add users KCM

cblack updated this revision to Diff 82012. cblack added a comment. Use section headers instead of font weights REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82010&id=82012 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kd

D28154: Add users KCM

ngraham added a comment. The avatar image bug is not fixed for me. :( REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D28154 To: cblack, #plasma, #vdg, ngraham Cc: iasensio, meven, crossi, The-Feren-OS-Dev, davidedmundson, broulik, filipf, ngraham, nicolasfell

D28154: Add users KCM

cblack updated this revision to Diff 82010. cblack added a comment. Improved dirty state handling REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82008&id=82010 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154

D28154: Add users KCM

ngraham added a comment. Great, those are fixed now. Here's an interesting new one I just found: when you change another user's avatar for the first time after opening the KCM (this is important), the change doesn't take effect until you change your own avatar too: F8287316: vokoscreenNG-202

D28154: Add users KCM

cblack updated this revision to Diff 82008. cblack added a comment. Address the page oddities REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=82006&id=82008 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154 AFFE

D28154: Add users KCM

ngraham added a comment. When I create a second user, the users list highlights that other user, but the details view shows me: F8287247: Screenshot_20200505_26.png Oh and here's a fun one for you: F8287249: vokoscreenNG-2020-05-05_11-12-54.web

D28154: Add users KCM

cblack updated this revision to Diff 82006. cblack added a comment. Take care of that feedback REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=80942&id=82006 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154 AFF

D28154: Add users KCM

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Various bugs I've complained about are still not fixed. 1. The KCM does not display the two-column view when opened in System Settings' default window size until you resize

D28154: Add users KCM

cblack added inline comments. INLINE COMMENTS > ngraham wrote in main.qml:88 > Not sure it's actually a bug in the multi-page KCM system as @iasensio was > able to make this work properly in D28152 > . Maybe you can see what he did there? Seems to work for m

D28154: Add users KCM

cblack updated this revision to Diff 80942. cblack marked 11 inline comments as done. cblack added a comment. Address feedback, needs fix for https://bugs.kde.org/show_bug.cgi?id=420439 in order to function REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.

D28154: Add users KCM

ngraham added inline comments. INLINE COMMENTS > UserDetailsPage.qml:195 > +onClicked: { > +if (kcm.deleteUser(usersDetailPage.user.uid)) { > +kcm.pop() This no longer asks if you want to delete that user's home directory, so it's alwa

D28154: Add users KCM

ngraham added inline comments. INLINE COMMENTS > user_manager.desktop:9 > +X-KDE-Weight=40 > +X-KDE-System-Settings-Parent-Category=personalization > +Name=Users You've moved it out of the Account details group. If this is intentional, then we also need to move the KWallet KCM out of that group

D28154: Add users KCM

ngraham added a subscriber: iasensio. ngraham added a comment. @meven this is what it looks like now: F8249229: Screenshot_20200420_074955.png @cblack Mostly okay now, just a few more broken things that need fixing. INLINE COMMENTS > UserDetailsPag

D28154: Add users KCM

meven added a comment. Is the screenshot current ? ping @ngraham REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D28154 To: cblack, #plasma, #vdg, ngraham Cc: meven, crossi, The-Feren-OS-Dev, davidedmundson, broulik, filipf, ngraham, nicolasfella, zzag, p

D28154: Add users KCM

cblack updated this revision to Diff 79605. cblack marked 2 inline comments as done. cblack added a comment. Improved avatar sheet REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79596&id=79605 BRANCH arcpatch-D28154_1 REVISION DETAIL h

D28154: Add users KCM

ngraham added inline comments. INLINE COMMENTS > UserDetailsPage.qml:267 > +id: imgDelegate > +smooth: true > + Despite this, they're still jaggy for me. Maybe play with the mipmapping settings? > cblack wrote in main.qml:88 > Seems to be

D28154: Add users KCM

cblack marked 2 inline comments as done. cblack added inline comments. INLINE COMMENTS > ngraham wrote in main.qml:88 > Not actually fixed. Just open System Settings with its default window size > and navigate to the new KCM and you'll see what I'm talking about. Seems to be a bug with syse/mul

D28154: Add users KCM

cblack updated this revision to Diff 79596. cblack added a comment. Move columWidth set to KCM constructor REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79593&id=79596 BRANCH arcpatch-D28154_1 REVISION DETAIL https://phabricator.kde.o

D28154: Add users KCM

cblack updated this revision to Diff 79593. cblack added a comment. Add border to button on details page REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79592&id=79593 BRANCH arcpatch-D28154_1 REVISION DETAIL https://phabricator.kde.org

D28154: Add users KCM

cblack updated this revision to Diff 79592. cblack added a comment. Add circle to accounts in the list REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79250&id=79592 BRANCH arcpatch-D28154_1 REVISION DETAIL https://phabricator.kde.org/D

D28154: Add users KCM

cblack updated this revision to Diff 79250. cblack marked 2 inline comments as done. cblack added a comment. Address some more issues REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79248&id=79250 BRANCH arcpatch-D28154 REVISION DETAIL

D28154: Add users KCM

ngraham added a comment. In D28154#640943 , @cblack wrote: > Add parent category Now it shows up on the base level of navigation, rather than within the Account Details group where it currently lives. REPOSITORY R119 Plasma Desktop RE

D28154: Add users KCM

cblack updated this revision to Diff 79248. cblack added a comment. Trim file:// off of icon string REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79245&id=79248 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154

D28154: Add users KCM

cblack updated this revision to Diff 79245. cblack added a comment. Add parent category REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79242&id=79245 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154 AFFECTED F

D28154: Add users KCM

ngraham added inline comments. INLINE COMMENTS > cblack wrote in ChangePassword.qml:65 > that's basically what this does Well, it doesn't work for me. :) REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D28154 To: cblack, #plasma, #vdg, ngraham Cc: crossi, The-Fe

D28154: Add users KCM

ngraham added inline comments. INLINE COMMENTS > UserDetailsPage.qml:101 > +layer.enabled: user.faceValid > +layer.effect: OpacityMask { > +maskSource: Rectangle { Nice! How about an outline around the circle like we do in Kickoff?

D28154: Add users KCM

cblack updated this revision to Diff 79242. cblack marked 3 inline comments as done. cblack added a comment. Address issues REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79241&id=79242 BRANCH arcpatch-D28154 REVISION DETAIL https://ph

D28154: Add users KCM

cblack updated this revision to Diff 79241. cblack marked 3 inline comments as done. cblack added a comment. Improve deletion and creation REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79240&id=79241 BRANCH arcpatch-D28154 REVISION DETA

D28154: Add users KCM

cblack updated this revision to Diff 79240. cblack added a comment. Drop anchors REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79236&id=79240 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154 AFFECTED FILES

D28154: Add users KCM

cblack added inline comments. INLINE COMMENTS > ngraham wrote in ChangePassword.qml:65 > hitting the return key on this field should effectively click the Create > button that's basically what this does REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D28154 To:

D28154: Add users KCM

ngraham added a comment. Now it's this: "file:///home/nate/kde/usr/share/kpackage/kcms/user_manager/contents/ui/UserDetailsPage.qml" "Error loading QML file.\n257: Type ChangePassword unavailable\n31: Cannot assign to non-existent property \"anchors\"\n" REPOSITORY R119 Plasm

D28154: Add users KCM

cblack updated this revision to Diff 79236. cblack added a comment. Remove modal REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79184&id=79236 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154 AFFECTED FILES

D28154: Add users KCM

ngraham added a comment. I think that broke the details page. Console spew says: QQmlComponent: Component is not ready "file:///home/nate/kde/usr/share/kpackage/kcms/user_manager/contents/ui/UserDetailsPage.qml" "Error loading QML file.\n257: Type ChangePassword unavailable\n

D28154: Add users KCM

cblack updated this revision to Diff 79184. cblack marked 3 inline comments as done. cblack added a comment. Address some feedback REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79181&id=79184 BRANCH arcpatch-D28154 REVISION DETAIL htt

D28154: Add users KCM

ngraham added inline comments. INLINE COMMENTS > ChangePassword.qml:65 > + > +onAccepted: !passwordWarning.visible && verifyField.text && > passwordField.text && passButton.apply() > + hitting the return key on this field should effectively click the Create button > UserDetails

D28154: Add users KCM

cblack updated this revision to Diff 79181. cblack marked 3 inline comments as done. cblack added a comment. Use better model roles REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79180&id=79181 BRANCH arcpatch-D28154 REVISION DETAIL ht

D28154: Add users KCM

cblack updated this revision to Diff 79180. cblack marked an inline comment as done. cblack added a comment. Address feedback REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79173&id=79180 BRANCH arcpatch-D28154 REVISION DETAIL https://

D28154: Add users KCM

cblack updated this revision to Diff 79173. cblack added a comment. Use a KJob to apply user configuration REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79167&id=79173 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org

D28154: Add users KCM

broulik added inline comments. INLINE COMMENTS > user.cpp:179 > +{ > +QtConcurrent::run([=]() { > +m_dbusIface->SetUserName(mName).waitForFinished(); No, just no. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D28154 To: cblack, #plasma, #vdg, ngrah

D28154: Add users KCM

cblack updated this revision to Diff 79167. cblack added a comment. Run apply in a separate thread REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79165&id=79167 BRANCH arcpatch-D28154 REVISION DETAIL https://phabricator.kde.org/D28154

D28154: Add users KCM

cblack updated this revision to Diff 79165. cblack marked 2 inline comments as done. cblack added a comment. Address feedback REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79138&id=79165 BRANCH arcpatch-D28154 REVISION DETAIL https://

D28154: Add users KCM

cblack added a comment. In D28154#640206 , @davidedmundson wrote: > > Drop group stubs; there will be bad UX because two different polkit actions (and thus two dialogs backtoback) would be needed for apply with groups > > Don't rule out the

D28154: Add users KCM

davidedmundson added a comment. > Drop group stubs; there will be bad UX because two different polkit actions (and thus two dialogs backtoback) would be needed for apply with groups Don't rule out the obvious other option there - changing accountsservice. It already has all the code, jus

D28154: Add users KCM

ngraham added a comment. So it does. My bad, sorry. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D28154 To: cblack, #plasma, #vdg, ngraham Cc: crossi, The-Feren-OS-Dev, davidedmundson, broulik, filipf, ngraham, nicolasfella, zzag, plasma-devel, Orage, LeGas

D28154: Add users KCM

broulik added a comment. That gallery comes from user manager right now, doesnt it? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D28154 To: cblack, #plasma, #vdg, ngraham Cc: crossi, The-Feren-OS-Dev, davidedmundson, broulik, filipf, ngraham, nicolasfella,

D28154: Add users KCM

ngraham added a comment. Wait, why are you adding the Konqis to this diff? You should use whatever framework provides the gallery instead of re-implementing all of that and duplicating the assets. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D28154 To: cb

D28154: Add users KCM

cblack updated this revision to Diff 79138. cblack marked 7 inline comments as done. cblack added a comment. Address feedback REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28154?vs=79099&id=79138 BRANCH arcpatch-D28154 REVISION DETAIL https://

D28154: Add users KCM

crossi added inline comments. INLINE COMMENTS > kcm.cpp:73 > + > +void KCMUser::defaults() > +{ Not necessary to override if you don't add your specific implementatioin. > kcm.cpp:78 > + > +void KCMUser::load() > +{ same here > user.cpp:53 > +mName = value; > +nameChanged(value); > +}

D28154: Add users KCM

ngraham added a comment. You should set `smooth: true` on those avatars in the new gallery so they aren't so jaggy. Also the button to add your own avatar from a file should probably have a label in it so people can figure out what it does. And the sheet that the new gallery is on should be

  1   2   >