TextBox Regex Extension #799
Conversation
|
|
||
| public object ConvertBack(object value, Type targetType, object parameter, string language) | ||
| { | ||
| throw new NotImplementedException(); |
There was a problem hiding this comment.
Please convert back too.
| <StackPanel Grid.Row="2" | ||
| Margin="10,0,10,0"> | ||
| <TextBox Name="EmailValidatorForce" | ||
| controls:TextBoxRegexEx.Regex="^([\w\.\-]+)@([\w\-]+)((\.(\w){2,3})+)$" |
There was a problem hiding this comment.
Consider making an enum option too. Regex.Email, Regex.Phone etc etc.
| Style="{StaticResource TextBoxRegexExStyle}" | ||
| Text="" /> | ||
| <StackPanel Orientation="Horizontal"> | ||
| <TextBlock Text="Validation Result: " /> |
There was a problem hiding this comment.
Change the text to "Is Valid", or "Valid Value" or similar. With "Validation Result" I would expect values of "Valid" and "Invalid"
| <TextBox Name="PhoneNumberValidatorForce" | ||
| controls:TextBoxRegexEx.ValidationMode="Forced" | ||
| controls:TextBoxRegexEx.ValidationType="PhoneNumber" | ||
| Header="Text box with ValidationType= PhoneNumber , validation occurs on textbox changed and force occurs on lose focus with ValidationMode=Force" |
There was a problem hiding this comment.
fix spacing before and after PhoneNumber
| Header="Text box with Regex extension for phone number, validation occurs on textbox changed" | ||
| HeaderTemplate="{StaticResource HeaderTemplate}" | ||
| Style="{StaticResource TextBoxRegexExStyle}" | ||
| Text="" /> |
There was a problem hiding this comment.
Remove the empty text setter
| <StackPanel Margin="10,0,10,0"> | ||
| <TextBox Name="PhoneNumberValidator" | ||
| controls:TextBoxRegexEx.Regex="^\s*\+?\s*([0-9][\s-]*){9,}$" | ||
| Header="Text box with Regex extension for phone number, validation occurs on textbox changed" |
There was a problem hiding this comment.
Should be "validation occurs on TextChanged."
| <TextBox Name="PhoneNumberValidatorForce" | ||
| controls:TextBoxRegexEx.ValidationMode="Forced" | ||
| controls:TextBoxRegexEx.ValidationType="PhoneNumber" | ||
| Header="Text box with ValidationType= PhoneNumber , validation occurs on textbox changed and force occurs on lose focus with ValidationMode=Force" |
There was a problem hiding this comment.
Should be "validation occurs on TextChanged and"
| /// </summary> | ||
| /// <param name="obj">TextBox Control</param> | ||
| /// <returns>Regex Value</returns> | ||
| public static string GetRegex(DependencyObject obj) |
There was a problem hiding this comment.
method parameter for the get and set methods should be TextBox
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets ValidationMode property |
There was a problem hiding this comment.
Change comments to follow pattern in previous comments
| private const string PhoneNumberRegex = @"^\s*\+?\s*([0-9][\s-]*){9,}$"; | ||
| private const string CharactersRegex = "^[A-Za-z]+$"; | ||
|
|
||
| private static void TextBoxRegexExPropertyOnChange(DependencyObject sender, DependencyPropertyChangedEventArgs e) |
There was a problem hiding this comment.
Make sure all occurrences of TextBoxRegexEx are replaced with TextBoxRegex
| textbox.TextChanged -= Textbox_TextChanged; | ||
| textbox.Loaded += Textbox_Loaded; | ||
| textbox.LostFocus += Textbox_LostFocus; | ||
| textbox.TextChanged += Textbox_TextChanged; |
There was a problem hiding this comment.
Should attempt to validate whenever the expression changes
| regex = textbox.GetValue(RegexProperty) as string; | ||
| if (string.IsNullOrWhiteSpace(regex)) | ||
| { | ||
| throw new ArgumentException("Regex property can't be null or empty when custom mode is selected"); |
There was a problem hiding this comment.
Just set IsValid to true and return. Don't think this warrants an exception
There was a problem hiding this comment.
If not exception is thrown then nothing will indicate there is something wrong :) if the user got to this scenario then he is making something wrong and that's why we should give a warning.
There was a problem hiding this comment.
Put a message to the output/debug window. Same as the way failed bindings do
|
Missed one: The number/decimal regex should probably allow for optional negative sign at the front |
| /// TextBoxRegex allows text validation using a regular expression. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// If<see cref="ValidationMode"> is set to Normal then IsValid will be set according to either the regex is valid.</see> |
There was a problem hiding this comment.
Make sure you proof my comments 😛
"set according to whether the regex is valid."
There was a problem hiding this comment.
the current written comment is right :)
There was a problem hiding this comment.
Also need a space after the first word 'If'
| /// <summary> | ||
| /// The regex expression that will be validated on the textbox | ||
| /// </summary> | ||
| public static readonly DependencyProperty RegexProperty = DependencyProperty.RegisterAttached("Regex", typeof(string), typeof(TextBoxRegexEx), new PropertyMetadata(null, TextBoxRegexExPropertyOnChange)); |
There was a problem hiding this comment.
Yes, the pattern above is for attached properties. See Grid.Row:
//
// Summary:
// Identifies the Grid.Row XAML attached property.
//
// Returns:
// The identifier for the Grid.Row XAML attached property.
GetRow and SetRow then have their comments of "Gets... " and "Sets..."
| /// </remarks> | ||
| public partial class TextBoxRegex | ||
| { | ||
| /// <summary> |
There was a problem hiding this comment.
summary tags for attached dependency properties should follow the format
/// <summary>
/// Identifies the PropertyNameHere attached dependency property.
/// </summary>
| /// </summary> | ||
| /// <param name="obj">TextBox to get Regex property from.</param> | ||
| /// <returns>The regular expression assigned to the TextBox</returns> | ||
| public static string GetRegex(DependencyObject obj) |
There was a problem hiding this comment.
Method parameter for the Get* and Set* methods should be a TextBox
| public static readonly DependencyProperty ValidationTypeProperty = DependencyProperty.RegisterAttached("ValidationType", typeof(ValidationType), typeof(TextBoxRegex), new PropertyMetadata(ValidationType.Custom, TextBoxRegexPropertyOnChange)); | ||
|
|
||
| /// <summary> | ||
| /// Gets the Regex property |
There was a problem hiding this comment.
Gets the value of the TextBoxRegEx.RegEx XAML attached property from the specified TextBox.
Following pattern from framework team. See Grid.Row, RelativePanel.Above, TooltipService.Tooltip, etc.
| } | ||
|
|
||
| /// <summary> | ||
| /// Set Regex property |
There was a problem hiding this comment.
Sets the value of the TextBoxRegEx.RegEx XAML attached property for a target TextBox.
Following pattern set by framework team
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets a value indicating if the Text is valid according to the Regex property. |
There was a problem hiding this comment.
Change any other Get* Set* summary tags to match pattern set by framework team
|
Flagging this one as 1.3 as I feel we are close |
|
@skendrot next time will not tell you that I'm doing a PR |
|
Congrats! |
referencing issue #753 for creating a validation regex extension for textbox.
As usual documentation is on the way :)