<table><tr><td style="">antlarr added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D2874" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>Thanks for your comments. I should warn you that this is the second time I touch qml code, so apologies in advance :).</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D2874#inline-11164" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">SessionManagementScreen.qml:60</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">this component is shared with the lockscreen and user switcher, you're going to be generating warnings there.</p>

<p style="padding: 0; margin: 8px;">but from the other comment, we don't actually want this check here anyway.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">ok</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D2874#inline-11163" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">SessionManagementScreen.qml:69</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Even when we're not showing the users we still want this view, as this view is set to a single entry model with the display name "Login as a different user"</p>

<p style="padding: 0; margin: 8px;">we always want to show this.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Hmm, the way I saw it, if showUserList is false (because there are more users than the threshold), the "Login as a different user" button is not visible, so there's no way to get the "Login as a different user" component on top of the stack view and so, there's no need for this to display anything.</p>

<p style="padding: 0; margin: 8px;">But from your comment I assume userListView is used by kscreenlocker for something else, and that's the reason you said its visibility can't depend on the user threshold to disable avatars, is that correct?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D2874#inline-11165" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">Login.qml:11</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">can you move the SDDM specific code here please.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I can move it here if you think it's better to implement option 1 (see next comment) or if there's a way to change the visibility of userListView from here.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D2874#inline-11166" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">Main.qml:114</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Why? If we're not showing the username prompt we won't be loading this component in the main view anyway.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Note that before I started working on all this I saw two ways to do this commit:</p>

<p style="padding: 0; margin: 8px;">Option 1) Use the userPromptComponent (the "Login as different user" view), add buttons to suspend/reboot/shutdown, remove the "back" button which returns to the initial view , change the top put this view at the top of the stack, and cross my fingers so that if the userlist has visible: true but is not at the top of the stack, the avatars are not actually loaded.</p>

<p style="padding: 0; margin: 8px;">Option 2) Use the userListComponent (the default view), making the showUsernamePrompt true, removing the "Login as different user" option (since it's basically the same). And that last part is what the line above does.</p>

<p style="padding: 0; margin: 8px;">Option 1 means duplicating much code (the whole actionItems section) and I guess there's a way to extract that somewhere and reuse the actionItems section in both userPromptComponent and a new fullFeaturedUserPromptComponent, but I don't know that much QML to do that, so I choosed option 2. If you can spend some time explaining how to do it that way, I don't mind changing that.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>rPLASMAWORKSPACE Plasma Workspace</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D2874" rel="noreferrer">https://phabricator.kde.org/D2874</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>antlarr, Plasma<br /><strong>Cc: </strong>davidedmundson, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas<br /></div>