Clarify date value in Pages
#61709
Merged
Clarify date value in Pages
#61709
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
oandregal
added
[Type] Enhancement
oandregal
requested review from
mcsf,
youknowriad,
jameskoster,
jorgefilipecosta and
ntsekouras
oandregal
changed the title
date date is displayed in Pagesdate value in Pages
|
Size Change: +239 B (+0.01%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
ntsekouras
reviewed
May 16, 2024
| ? item.modified | ||
| : item.date; | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
I think we need to use createInterpolateElement in these cases.. cc @mcsf
There was a problem hiding this comment.
Thanks for bringing it up. I was looking into that but had issues with collapsed whitespace, note how the Scheduled:May here:
It turns out it wasn't the interpolation but a CSS rule (display: flex) of the dataviews-view-table__cell-content-wrapper. I just figured the text node was considered a different node and its whitespace collapsed, so I added a span wrapper for the text node & the time to be considered a single element by its flex container.
ntsekouras
reviewed
May 16, 2024
| const formattedDate = dateI18n( | ||
| getSettings().formats.datetimeAbbreviated, | ||
| getDate( item.date ) | ||
| const isDraftOrPrivate = [ 'draft', 'private' ].some( |
There was a problem hiding this comment.
Nit: we could just use Array.includes here. Also no need to have it as a variable.
ntsekouras
reviewed
May 16, 2024
| if ( isPublished ) { | ||
| const isModified = | ||
| getDate( item.modified ) > getDate( item.modified ); | ||
| const dateToDisplay = isModified |
There was a problem hiding this comment.
Nit: I guess we could reuse the code to check if is modified. I don't think we care about the order of checks here, since we check against explicit post statuses, so the modified check could be grouped together..
There was a problem hiding this comment.
Organized the code a bit differently, so it's clear with post statuses differentiate between modified/creation date and which ones do not.
Feedback addressed. I've also added a comparison between wp-admin and dataviews in the PR description:
|
bph
pushed a commit
to bph/gutenberg
that referenced
this pull request
May 27, 2024
Co-authored-by: oandregal <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: jasmussen <[email protected]> Co-authored-by: jameskoster <[email protected]>
|
I have a follow-up #64049 The Published posts need to use the published date, not the modified one. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #60477
What?
Enhances the information displayed in the
datefield.Why?
We aim to get parity with wp-admin:
Target design for dataviews:
How?
Adds logic to the
datefield rendering depending on the post status:item.date) or modified (item.modified).item.date) date, which is in the future.item.date), even if it has been modified.item.date) or modified (item.modified).Testing Instructions