Support timeFormat in datetime field

[nhatrangnet]

https://app.asana.com/1/11768486970802/project/1200179452020411/task/1212702601974424

[rilwis]

Còn thiếu phần unparse nữa. E test cả với import export, save to json nhé.

[opencode-agent]

Based on my analysis of the pull request, here's my review:

Code Review Summary

✅ Good Implementation

• Parser (src/Parsers/Field.php:291-321): Correctly splits datetime_format into dateFormat, timeFormat, and separator in js_options
• Unparser (src/Unparsers/Field.php:277-303): Properly reverses the parsing, reconstructing datetime_format from js_options
• Both methods follow existing code patterns and naming conventions
• Block editor field support added consistently

---

⚠️ Issues Found

1. Bug: Empty js_options handling in unparser (src/Unparsers/Field.php:278-280)

if ( empty( $this->js_options ) ) {
    return $this;
}

If js_options is an empty array [], this passes through but later code at line 295 will fail because it's expecting an array of objects format. Should check ! is_array($this->js_options) or use null !== $this->js_options.

2. Missing type hint on get_js_option_value (src/Unparsers/Field.php:305)

private function get_js_option_value( array $js_options, string $key ) {

Should add return type: : ?string for PHP 7.4 compatibility.

3. Redundant separator setting (src/Parsers/Field.php:312-314)

if ( empty( $js_options['separator'] ) ) {
    $js_options['separator'] = ' ';
}

The $separator variable is already set to ' ' as default on line 297, so this always sets it. Either remove this check or move it before line 297.

4. Inconsistent variable naming (src/Parsers/Field.php:297):

$separator  = $js_options['separator'] ?? ' ';

vs

$separator   = $this->get_js_option_value( $js_options, 'separator' ) ?? ' ';

The parser uses direct key access while unparser uses helper function. This inconsistency could cause issues if the format changes.

5. PHPDoc missing for new methods
Add @return self for parse_field_datetime(), parse_field_block_editor(), unparse_field_datetime(), and unparse_field_block_editor().

---

📝 Minor Suggestions

• Consider extracting the datetime format parsing logic into a reusable trait if similar patterns exist elsewhere
• The uniqid() in unparse_field_block_editor:262 as fallback is fine but could produce unpredictable names

---

🔒 Security

No security issues found. The code doesn't handle user input directly and uses proper escaping/context.

opencode session  |  github run