Password show hiden feature #14648
Password show hiden feature #14648
Conversation
|
|
||
| public const int MaxPasswordLength = 128; | ||
|
|
||
| public const int AdminEmailAddress = 256; |
There was a problem hiding this comment.
rename to MaxAdminEmailLength is better. and why here use const not static?
or can we reuse AbpUserConsts.MaxEmailLength ?:
just like:
There was a problem hiding this comment.
- I need to the const value because it should be changed
- Attributes accept just const value
There was a problem hiding this comment.
const value will not allow developer to change outside of module.
use DynamicStringLengthAttibute, see #4496
| @@ -127,4 +126,9 @@ protected override ValueTask SetTableColumnsAsync() | |||
|
|
|||
| return base.SetTableColumnsAsync(); | |||
| } | |||
|
|
|||
| protected virtual void ShowHidePassword(bool isShowPassword) | |||
There was a problem hiding this comment.
no need parameter isShowPassword here.
simple:
protected virtual void ShowHidePassword()
{
IsShowPassword = !IsShowPassword;
}There was a problem hiding this comment.
It's simpler but not a safe behaviour
There was a problem hiding this comment.
sorry, can you explain what's mean of not a safe behaviour
There was a problem hiding this comment.
yw, to avoid multiple instances
| @@ -228,16 +229,21 @@ protected override ValueTask SetToolbarItemsAsync() | |||
| return base.SetToolbarItemsAsync(); | |||
| } | |||
|
|
|||
| protected virtual void ChangePasswordTextRole(TextRole? textRole) | |||
| protected virtual void ChangePasswordTextRole(TextRole? textRole, bool? isShowPassword = null) | |||
There was a problem hiding this comment.
no need isShowPassword parameter here. Can use IsShowPassword property directly in this function
There was a problem hiding this comment.
It's simpler but not a safe behaviour
There was a problem hiding this comment.
It seems isShowPassword isn't necessary as a method parameter. That value is already known in the class.
| @@ -35,12 +35,13 @@ public partial class UserManagement | |||
| protected string CreateModalSelectedTab = DefaultSelectedTab; | |||
|
|
|||
| protected string EditModalSelectedTab = DefaultSelectedTab; | |||
| protected bool IsShowPassword { get; set; } | |||
There was a problem hiding this comment.
The name of the property is confusing. It can be IsPasswordShown or ShowPassword
| @@ -228,16 +229,21 @@ protected override ValueTask SetToolbarItemsAsync() | |||
| return base.SetToolbarItemsAsync(); | |||
| } | |||
|
|
|||
| protected virtual void ChangePasswordTextRole(TextRole? textRole) | |||
| protected virtual void ChangePasswordTextRole(TextRole? textRole, bool? isShowPassword = null) | |||
There was a problem hiding this comment.
It seems isShowPassword isn't necessary as a method parameter. That value is already known in the class.
closes https://github.com/volosoft/volo/issues/10076