Editor: Enhance classic editor timestamp fields with native controls (continues #12275) - #13050
Editor: Enhance classic editor timestamp fields with native controls (continues #12275)#13050j111q wants to merge 3 commits into
Conversation
Follow-up to the review on PR WordPress#12275 by @tyxla: - Remove a duplicated `.misc-pub-curtime #timestamp:before` rule, keeping the later declaration so it still overrides `.curtime #timestamp:before` (equal specificity, so source order decides). - Align the native date/time invalid-state selectors: mark and clear `form-invalid` on both native inputs via `.timestamp-native-wrap input`, instead of flagging only `#publish-date-native` by ID. - Render a zero UTC offset as `UTC` rather than `UTC+0` in the site-time note. - Size the Publish-box timestamp icon column with `auto` instead of a fixed `27px`, so the row's label lines up with the sibling meta rows. Props poligilad, tyxla. See WordPress#12275.
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Nice, thanks so muc for contributing! From a quick glance it looks good to me, but honestly I'll also defer a bit to you and Poli on the design side. I understand this is motivated by good reasons, and thus I mostly want to support it with my own thumbs up. The changeset looks reasonable to me, so I'm wondering: how can we help this ship? Let's see if some developers check in on this with input, otherwise if nothing happens, ping me again and we'll try and see if a |
…pat. Wrapping the timestamp display and the Edit link in a `.timestamp-display` span moved `a.edit-timestamp` out from being a sibling of `#timestampdiv`. A scan of the plugin directory found several plugins (e.g. PublishPress Statuses, Media Library Assistant, LH Archived Post Status) that rely on `$( '#timestampdiv' ).siblings( 'a.edit-timestamp' )`, which the wrapper would break. Keep `#timestamp` and `a.edit-timestamp` as direct children of `.misc-pub-curtime` and lay the row out with a three-column grid instead. The rendered result and the row alignment are unchanged. See WordPress#12275.
|
Thanks @jasmussen (and for the quick reply)! 🙏 @tyxla -- following up on your point about not breaking plugins that integrate with the existing controls. To investigate this, I was directed by Claude to run the change through WPDirectory (regex across all current plugins + themes on .org) The results:
So the CSS changes look safe, and no themes are affected. The one real compatibility surface is the JS: wrapping the display + Edit link moved Triaging the ~14 hits:
Mitigation which has been pushed: rather than accept even a small break, I kept The rendered result and alignment are unchanged; Pls LMK if there's something else you think I could do to ensure backward compatibility here! 🙏 |
tyxla
left a comment
There was a problem hiding this comment.
Thank you for working on it @j111q!
The biggest piece of feedback I have is related to the fact that this is impacting more screens than it aims to. A more secondary issue is that it causes some visual issues when the form is collapsed.
See inline comments.
Also I'd encourage you to test with different languages, especially ones with longer labels (German, Finnish), also with RTL ones.
| height: auto !important; | ||
| } | ||
|
|
||
| .misc-pub-curtime { |
There was a problem hiding this comment.
Note that this classname is used in more places than where this PR assumes:
- The attachment submit box ("Uploaded on:")
- The comment edit screen ("Submitted on:")
And a quick test reveals this is broken for the default comment on a fresh install:
This may also mean there might be broader impact than we think (both core-wise and plugin-wise).
There was a problem hiding this comment.
I think we need to reserve space for the Edit link here. With a longer translated timestamp, the max-content column takes most of the available width and pushes Edit outside the metabox.
|
|
||
| // Cancel submit when an invalid timestamp has been selected. | ||
| $('#post').on( 'submit', function( event ) { | ||
| if ( ! updateText() ) { |
There was a problem hiding this comment.
If a user clears a native date or time field and clicks the main Publish/Update button, the old legacy values are still valid, so the form submits the old timestamp. Haven't tested, but maybe we should call updateFieldsFromNativeTimestamp() here instead of updateText().
|
|
||
| .misc-pub-curtime { | ||
| display: grid; | ||
| grid-template-columns: auto minmax( 0, max-content ) minmax( 0, 1fr ); |
There was a problem hiding this comment.
This is where we need to reserve space for the Edit link. With a longer translated timestamp, the max-content column takes most of the available width and pushes Edit outside the metabox. Perhaps auto minmax( 0, 1fr ) auto?
| min-width: 0; | ||
| } | ||
|
|
||
| .misc-pub-curtime:before { |
There was a problem hiding this comment.
Should this new pseudo-element also use: #8c8f94? The clock icon moved here, but the existing color rule still targets #timestamp:before, which is now disabled. So the clock is darker than the other icons in the publish box.
| } | ||
|
|
||
| .misc-pub-curtime { | ||
| display: grid; |
There was a problem hiding this comment.
This will be one more thing to check for back-compat: this element is now a grid container. Any element that a plugin adds as a direct child becomes a grid item and moves to a new row, instead of flowing inline as before. This may be harder to catch with https://wpdirectory.net/ FWIW.
| return false; | ||
| } | ||
|
|
||
| $( '#aa' ).val( dateMatches[1] ); |
There was a problem hiding this comment.
.val() does not fire a change event. So we might want to explicitly .trigger( 'change ).
| $timestampwrap.hide(); | ||
| $nativeTimestampWrap.removeAttr( 'hidden' ); | ||
| $nativeTimestampWrap.find( 'input' ).on( 'change', updateFieldsFromNativeTimestamp ); | ||
| $timestampwrap.find( 'input, select' ).on( 'change', updateNativeTimestampFields ); |
There was a problem hiding this comment.
Related to the above, the .val() does not trigger change so this handler probably never runs
| $cur_hh = current_time( 'H' ); | ||
| $cur_mn = current_time( 'i' ); | ||
|
|
||
| $timezone = wp_timezone_string(); |
There was a problem hiding this comment.
We call this every time now, but it's not used on the QuickEdit or comment screen. Should we move it where it's used? The if ( $for_post && ! $multi ) check.
| if ( $for_post && ! $multi ) { | ||
| ?> | ||
| <div class="timestamp-native-wrap hide-if-no-js" hidden> | ||
| <label for="publish-date-native" class="screen-reader-text"><?php _e( 'Date' ); ?></label> |
There was a problem hiding this comment.
These two labels need a /* translators: Hidden accessibility text. */ comment above the strings, like the other screen reader strings in this function.
| printf( | ||
| /* translators: %s: The site's timezone. */ | ||
| __( 'Site time: %s' ), | ||
| '<span>' . esc_html( $timezone ) . '</span>' |
Summary
Continues the work in #12275 (opened by @poligilad-auto, who is currently out) to enhance the classic editor Publish timestamp with server-rendered native
dateandtimecontrols, keeping the existing granularaa/mm/jj/hh/mnfields as the source of truth and fallback.This branch carries Poli's original commit unchanged and adds a follow-up commit addressing @tyxla's code review:
.misc-pub-curtime #timestamp:before { content: none }was declared twice; kept the later declaration so it still overrides.curtime #timestamp:before(equal specificity, so source order decides which wins).form-invalidclass is now added and removed on both native inputs via.timestamp-native-wrap input, rather than being added to only#publish-date-nativeby ID while being removed from both.UTCinstead ofUTC+0. A zero timezone offset now renders asUTCin the site-time note; non-zero offsets are unchanged (UTC+5:30, etc.).27px, which pushed the timestamp label ~10px further right than the Status / Visibility / Revisions rows. Sizing the column withautolets it match the sibling dashicon advance, so all rows line up. All new layout CSS remains scoped to.misc-pub-curtime, so the other meta boxes are unaffected.Trac ticket: https://core.trac.wordpress.org/ticket/61652
Still open from the review
The two higher-level points @tyxla raised on #12275 are not resolved here and still need discussion:
Confirming this is the design direction the project wants for the classic editor timestamp. @jasmussen - could you take a look?
Confirming the markup changes don't break plugins that integrate with the existing controls. -- I'll dig into this
Testing
dateandtimeinputs with the site-time note, and that the timestamp row aligns with the other Publish-box rows.php -l src/wp-admin/includes/template.php— no syntax errors.The follow-up commit was drafted with AI assistance and reviewed by me.
Props poligilad, tyxla.