antlarr added a comment.

  Thanks for your comments. I should warn you that this is the second time I 
touch qml code, so apologies in advance :).

INLINE COMMENTS

> davidedmundson wrote in SessionManagementScreen.qml:60
> this component is shared with the lockscreen and user switcher, you're going 
> to be generating warnings there.
> 
> but from the other comment, we don't actually want this check here anyway.

ok

> davidedmundson wrote in SessionManagementScreen.qml:69
> 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"
> 
> we always want to show this.

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.

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?

> davidedmundson wrote in Login.qml:11
> can you move the SDDM specific code here please.

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.

> davidedmundson wrote in Main.qml:114
> Why? If we're not showing the username prompt we won't be loading this 
> component in the main view anyway.

Note that before I started working on all this I saw two ways to do this commit:

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.

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.

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.

REPOSITORY
  rPLASMAWORKSPACE Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D2874

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: antlarr, #plasma
Cc: davidedmundson, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas

Reply via email to