-
Notifications
You must be signed in to change notification settings - Fork 99
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
PHPCS for TextArea and Datepicker #533
Conversation
Fixes: - Generic.Commenting.DocComment.MissingShort - Generic.Commenting.DocComment.SpacingBeforeTags - Generic.Files.EndFileNewline.NotFound - Squiz.Commenting.FileComment.Missing - Squiz.Commenting.FunctionComment.MissingParamComment - Squiz.Commenting.FunctionComment.SpacingAfterParamType
Fixes: - Generic.Commenting.DocComment.MissingShort - Generic.Commenting.DocComment.SpacingBeforeTags - Generic.ControlStructures.InlineControlStructure.NotAllowed - Generic.PHP.LowerCaseConstant.Found - Squiz.Commenting.FileComment.Missing - Squiz.Commenting.FunctionComment.MissingParamComment - Squiz.Commenting.FunctionComment.MissingParamName - Squiz.Commenting.FunctionComment.MissingParamTag - Squiz.Commenting.FunctionComment.ParamCommentFullStop - Squiz.Commenting.FunctionComment.ParamNameNoMatch - Squiz.Commenting.InlineComment.InvalidEndChar - WordPress.PHP.YodaConditions.NotYoda - WordPress.WhiteSpace.OperatorSpacing.NoSpaceAfter
*/ | ||
public function presave( $value, $current_value = array() ) { | ||
$time_to_parse = sanitize_text_field( $value['date'] ); | ||
if ( isset( $value['hour'] ) && is_numeric( $value['hour'] ) && $this->use_time ) { | ||
$hour = intval( $value['hour'] ); | ||
$minute = ( isset( $value['minute'] ) && is_numeric( $value['minute'] ) ) ? intval( $value['minute'] ) : 0; | ||
if ( $hour == 0 && $this->use_am_pm ) $hour = 12; | ||
if ( 0 == $hour && $this->use_am_pm ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this use the strict comparison?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly, but I don't want to risk changing the field's behavior in this PR, which is intended only to make the field standards-compliant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but I thought using a strict comparison was making it standards compliant :) Though it is only a warning so I guess it is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a problem with making that change now
Very minor comment, otherwise ⛳ |
See #271.