Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide username on LoginDialog #2230

Merged
merged 3 commits into from Dec 5, 2015
Merged

Conversation

mgnslndh
Copy link
Contributor

The LoginDialog can now be shown like this to hide the username and only ask for a password:

var result = await this.ShowLoginAsync("Authentication", "Enter your password", 
                                       new LoginDialogSettings 
                                       { 
                                           UsernameVisibility = Visibility.Collapsed
                                       });

Closes #2229

@punker76
Copy link
Member

@mgnslndh thx, what if you set toVisibility=Hidden? then you got a empty line. is this good? maybe a short bool like HideUsername could be better. what do you think?
/cc @thoemmi

@mgnslndh
Copy link
Contributor Author

Good point. I went for Visibility mostly because NegativeButton used it. But that does not mean it is appropriate for the Username of course.

HideUsername is better. True value results in Collapsed visibility, otherwise Visible.

@mgnslndh
Copy link
Contributor Author

@punker76 HideUsername would require a custom BooleanToVisibilityConverter that inverts conversion (true => Collapse and false => Visible).

Is this OK, or should we use another property name like ShowUsername or IsUsernameVisible instead?

One solution could be to use HideUsername in the LoginDialogSetting but use Visibility for the dependency property. What do you think?

@thoemmi
Copy link
Collaborator

thoemmi commented Nov 26, 2015

LoginDialogSettings should not have a property of type Visibility, that's an implementation detail. Use bool instead.

In general flags should be false by default, so IsUsernameHidden or ShouldHideUsername would be approriate. If that requires a new IConverter, well, it's easily written.

Collapsed is better than Hidden so no screen estate is preserved.

@punker76
Copy link
Member

@mgnslndh can you change your PR to the suggestions?

@mgnslndh
Copy link
Contributor Author

@punker76 @thoemmi I can change the PR and use a bool for the property on LoginDialogSettings, but I'd like your feedback on the dependency property type. Should it also be bool like on the LoginDialogSettings property or can it be of the type Visibility in which case we do not need a IValueConverter.

@mgnslndh
Copy link
Contributor Author

@punker76 I've addressed the suggestions. Does it look OK?

@punker76 punker76 added this to the 1.2.0 milestone Dec 5, 2015
@punker76 punker76 merged commit 5d6e1a3 into MahApps:master Dec 5, 2015
@punker76
Copy link
Member

punker76 commented Dec 5, 2015

@mgnslndh i merged your changes, but removed your converter and uses the new VisibilityHelper property IsCollapsed.

/cc @thoemmi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants