feature(DatePickerSetupControl): improved resolution of the alternati…#1307
Conversation
labmecanicatec
force-pushed
the
flatpickr
branch
from
1097604 to
c8c2ae7
Compare
There was a problem hiding this comment.
Pull request overview
This PR enhances DatePickerSetupControl by documenting its public inputs and refactoring how the control resolves and converts the alternate display format (AltFormat) for Flatpickr, including improved 24-hour time detection.
Changes:
- Add a comprehensive class docblock describing supported inputs and providing Smarty usage examples.
- Refactor alt-format handling to resolve
AltFormatas either a Resources key or a raw PHP date format, then convert it to Flatpickr format. - Introduce
ResolvePhpAltFormat()and baseTime24Hrdetection on the resolved PHP alt format.
labmecanicatec
force-pushed
the
flatpickr
branch
2 times, most recently
from
e7a6360 to
06f01bb
Compare
labmecanicatec
force-pushed
the
flatpickr
branch
from
06f01bb to
cc50eb8
Compare
labmecanicatec
force-pushed
the
flatpickr
branch
from
cc50eb8 to
78c41c5
Compare
labmecanicatec
force-pushed
the
flatpickr
branch
from
78c41c5 to
a61041c
Compare
| if ($isEscaped) { | ||
| // PHP and Flatpickr both use backslash to escape literals. | ||
| $result .= '\\' . $c; | ||
| $isEscaped = false; | ||
| continue; |
There was a problem hiding this comment.
When $isEscaped is true, this appends '\\' . $c, which evaluates to two backslashes plus the char. For Flatpickr escaping you typically want to emit a single backslash character before the literal (the JSON encoding step will take care of escaping it for JS). Consider appending a single backslash instead so escaped literals like \T round-trip correctly.
| if ($c === '\\') { | ||
| $isEscaped = true; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
if ($c === '\\') will never match because $c from str_split() is a single-character string (\), while '\\' evaluates to two backslashes. This prevents escape detection and will break formats that contain escaped literals (and also impacts the time/AMPM detection helpers below). Compare against a single backslash character instead.
There was a problem hiding this comment.
I think Copilot is wrong about this. '\\' is converted to a single backslash.
| } | ||
|
|
||
| if ($isEscaped) { | ||
| $result .= '\\\\'; |
There was a problem hiding this comment.
If the PHP format ends with a trailing backslash, $result .= '\\\\'; adds multiple backslashes (this literal evaluates to more than one \). Given Flatpickr escaping rules, this likely over-escapes; after fixing the single-backslash detection above, also adjust this to emit the intended number of backslashes (usually \\ in the Flatpickr format to represent a literal \).
| $result .= '\\\\'; | |
| $result .= str_repeat('\\', 2); |
| if ($c === '\\') { | ||
| $isEscaped = true; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
In HasTimeComponentInPhpFormat(), if ($c === '\\') won’t ever match a single backslash character from str_split(), so $isEscaped is never set. That makes escaped literals (e.g. \H, \A) incorrectly count as time/AMPM tokens. Update the check to compare against a single backslash.
| if ($c === '\\') { | ||
| $isEscaped = true; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Same escape-handling issue here: if ($c === '\\') will never match a single backslash from str_split(), so escaped literals aren’t skipped and a/A can be mis-detected even when escaped. Compare against a single backslash so $isEscaped works as intended.
…format and documentation. Add comprehensive class docblock and examples, and refactor alt-format handling for the DatePicker setup control. Introduce ResolvePhpAltFormat() to resolve AltFormat either as a resource key or raw PHP format, compute phpAltFormat and convert it to Flatpickr format, and base 24hr time detection on the resolved alt format. Also adjust default PHP alt format to include time when HasTimepicker is true and expose Time24Hr and DateFormat appropriately. Add HasDateFormat($key) to IResourceLocalization/Resources and use it from DatePickerSetupControl to resolve AltFormat without relying on the "?" fallback.
JohnVillalovos
force-pushed
the
flatpickr
branch
from
a61041c to
39023c8
Compare
JohnVillalovos
left a comment
JohnVillalovos
left a comment
There was a problem hiding this comment.
Thanks @labmecanicatec
LGTM
…ve format and documentation.
Add comprehensive class docblock and examples, and refactor alt-format handling for the DatePicker setup control. Introduce ResolvePhpAltFormat() to resolve AltFormat either as a resource key or raw PHP format, compute phpAltFormat and convert it to Flatpickr format, and base 24hr time detection on the resolved alt format. Also adjust default PHP alt format to include time when HasTimepicker is true and expose Time24Hr and DateFormat appropriately.