Enhance/display important dates on search #41301

millerf · 2020-04-20T18:18:22Z

Finishing Enhance/display important dates on search #41242 after a rebase

Changes proposed in this Pull Request

Before	After	Comment
		On listing, scheduled posts/pages now display the right date (scheduled ate)
		Published now display "sticky" if needed
		Post Search now displays the right status and appropriate date. The "sticky" status has been added. Maybe need some feedback from design?
		Pages search now displays only one date and the right status

Based on @sixhours remarks (#41242 (review)), some minor fixes need to be added.

Fixes #41075

Added "sticky" status for posts on search and listing

Add sticky Label for posts

36be7c5

client/my-sites/post-relative-time-status/index.jsx

millerf · 2020-04-20T20:46:30Z

@sixhours

When searching for a scheduled post, the line "Scheduled for Tomorrow at [time]" reads oddly because "tomorrow" is capitalized:

That cannot be reproduced on this branch... It shows:

But I think the latter format was better, as it showed the hour.

sixhours · 2020-04-20T20:53:53Z

I can't reproduce this behavior with this branch, either, but I can on production:

So maybe we add the post time back, ie. "Scheduled in a day at [time]"?

millerf · 2020-04-20T21:37:14Z

@sixhours I just pushed a fix.

client/my-sites/post-relative-time-status/index.jsx

millerf · 2020-04-21T09:24:50Z

I'm not a native English speaker

Me neither... I'll change "on" to "for" right away...

client/my-sites/post-relative-time-status/index.jsx

mmtr · 2020-04-21T10:01:47Z

Thanks @millerf for taking this one!

Some quick notes after testing the posts search.

Before (prod)	After (this branch)

Not sure if the comma after "Draft" or "Published" is intended but I think we can drop it.

Published posts should display the published date rather than the last modified date

Scheduled posts for tomorrow should include the "for" preposition as well.

Posts pending review should display the last modified date, along with the Pending review flag. I think we can consider them as drafts, since they have not been published yet.

millerf · 2020-04-21T10:07:39Z

Not sure if the comma after "Draft" or "Published" is intended but I think we can drop it.

It is intended, we discussed this here. @sixhours thought it was misleading.

Scheduled posts for tomorrow should include the "for" preposition as well.

Will do

Posts pending review

How do you create those ones? 😅
I remember we talked about it. I should be able blind-fix it anyway...

mmtr · 2020-04-21T10:16:09Z

It is intended, we discussed this here. @sixhours thought it was misleading.

Uhm, I think she was referring specifically to the published post status combined with the last modified date. If we change that to read "Published on [published_date]" as I suggested above, I don't think we need the comma.

How do you create those ones?

By toggling the "Pending review" checkbox on the sidebar panel and saving it as pending:

client/my-sites/post-relative-time-status/index.jsx

mmtr · 2020-04-28T08:05:24Z

client/my-sites/post-relative-time-status/index.jsx

+			extraStatusClassName = 'is-trash';
+			statusIcon = 'trash';
+			const displayedTime = this.getDisplayedTimeForLabel();
+			statusText = this.props.translate( 'trashed %(displayedTime)s', {


Cannot add more comments to the previous discussion, so I'm copying the comments over here to continue it.

@mmtr #41301 (comment):

@Automattic/i18n or @akirk can confirm, but I think we need to handle both status and date on the same translation. For instance, here displayScheduleTime contains on [date] for posts trashed more than a week ago, so trashed %(displayScheduleTime)s finally reads as trashed on [date]. Some languages might need to move the translated "on" part before "trashed" (while leaving the date after). To support those languages, the translatable string should be trashed on [date] rather than trashed [datePrefixedWithOn].

The same applies for the rest of statuses (scheduled for [date], published on [date], draft last modified on [date], ...).

@millerf #41301 (comment):

This is quite complex to tackle, as the "on" is not always present (for example : "Published yesterday" vs "Published on Mar, 23 march 2020")
And as translation texts cannot have variables to pass on the status, I would have to have to list all cases that could happen.

Also waiting to translators to confirm, but I think the "on" is the beginning of the proposition of time, so it correlates more with the date than the adjective. As in "On 23rd of March, I published, etc,..." But I might be mistaken...

Also waiting to translators to confirm, but I think the "on" is the beginning of the proposition of time, so it correlates more with the date than the adjective

Yeah, let's see if @Automattic/i18n or @akirk can jump in. I might be wrong as well, but languages are so different that I wouldn't assume that prepositions are always part of the date in all of them.

client/my-sites/post-relative-time-status/index.jsx

akirk · 2020-04-28T09:29:34Z

I took a look at the screenshots and the code and it seems to me we still have a lot of string concats here which should be avoided. In some languages maybe the date needs to be put at the start of the phrase, this makes it impossible. Also it's hard to know if in a translation the date might depend on the rest of the phrase. Since there are not that many possible states, I'd recommend to just write them out:

Trashed on %(specificDate)s (comment: Example: Trashed on March 3)
Trashed %(relativeTimeAgo)s (comment: Example: Scheduled 3 days ago)
Published on %(specificDate)s (comment: Example: Published on March 3)
Published %(relativeTimeAgo)s (comment: Example: Published 3 days ago)
Last modified on %(specificDate)s (comment: Example: Last modified on March 3)
Last modified %(relativeTimeAgo)s (comment: Example: Last modified 3 days ago)
Scheduled %(futureRelativeDateAndTime)s (comment: Example: Scheduled in 3 hours)
Scheduled for %(futureSpecificDateAndTime)s (comment: Example: Scheduled for March 3)

Where possible, we should go for a label, like with Pending Review, we should do the same for Private and Draft. So for example:

Scheduled in 2 minutes <icon> Pending Review
Scheduled for August 3 10:00 <icon> Private <icon> Sticky
Last modified 3 days ago <icon> Draft <icon> Private
Last modified on January 1 <icon> Draft <icon> Private
Last modified on January 1 <icon> Pending Review
Published 3 days ago <icon> Private <icon> Sticky
Published on January 1 <icon> Private <icon> Sticky
Trashed 3 days ago <icon> Private <icon> Sticky
Trashed on January 1 <icon> Private <icon> Sticky

Using a clock instead of "Last modified" and then the plain specific date or relative date is also an option. But it should only be used for "Last modified" and that date should only be displayed when the post is in the draft stage (resp. pending review).

mmtr · 2020-04-28T11:00:55Z

Where possible, we should go for a label, like with Pending Review, we should do the same for Private and Draft.

Not sure we need a Draft label. It is a status like trashed, published or scheduled. In fact, the last modified date will be only displayed on drafts, which is why we suggested to display posts in this state as Draft last modified on [date]/Draft last modified [relativeTimeAgo].

Fixes Automattic#41245

millerf · 2020-04-28T18:32:46Z

Also it's hard to know if in a translation the date might depend on the rest of the phrase

Following our discussion about the right place of the [on], please consider those examples:

Scheduled in 2 minutes
Scheduled for August 3 10:00
Published on January 1

[in], [for] and [on] have the exact same syntaxic/grammatical function. So if we had to move [on] to the "published" translation block following your logic, we should also move [for] and [in] (although the last one is directly translated from the moment library, so it might be difficult).

I'll prepare the code for that anyway, but I think we really need some translator to help out here before doing anything 😄

akirk · 2020-04-28T18:46:38Z

[in], [for] and [on] have the exact same syntaxic/grammatical function.

In English. You cannot make this assumption for other languages, some languages don't have prepositions. I only know a few but I can well see cases where a relative time needs to come at the beginning of a sentence, while an absolute time needs to be at the end. This is why we should not split sentences and join them through concat.

client/my-sites/post-relative-time-status/index.jsx

mmtr

Thanks again for the latest update, @millerf! I tried to test all the different combinations and it works great overall. I've found two issues, but only one of them is blocking. Please, see below.

⚠️ Published
Private posts are flagged twice now: with a prefix in the title and with a status label. Fine to address this in a follow-up.

Posts	Pages

✅Drafts

Posts	Pages

✅Scheduled

Posts	Pages

✅Trashed

Posts	Pages

❌Search
Published date is not displayed for private posts. We'd need to fix this before merging.

Posts	Pages

mmtr · 2020-04-29T09:54:07Z

client/my-sites/post-relative-time-status/index.jsx

+					} );
+				}
+				return this.props.translate( 'trashed %(displayedTime)s', {
+					comment: '%(displayedTime)s is when a post or page was trashed',


Given absolute/relative dates need to be at a different place for some languages, translators will need to know if this date is relative or absolute. Could you add some extra info in the comments so they can know that we use an absolute date in trashed on/published on/scheduled for/last modified on [date] and a relative date in trashed/published/last modified [date]?

millerf · 2020-04-29T16:06:44Z

@mmtr Thanks a lot for the deep testing. Yesterday I didn't quite finish what I wanted to do, to it is still work-in-progress. Sorry I forgot to mention it!

But anyway, I found something interesting:

Private posts are flagged twice now: with a prefix in the title and with a status label. Fine to address this in a follow-up.

I didn't have any private post (I was testing with pages), so I didn't see it.
It turns out it is coming from the back-end. They prefix the post title with "private: " when searching.

Request URL: https://public-api.wordpress.com/rest/v1.1/sites/174604573/posts?http_envelope=1&author=&number=20&order=DESC&search=&status=publish%2Cprivate&type=post&page=1

millerf · 2020-04-29T16:15:45Z

Published date is not displayed for private posts. We'd need to fix this before merging.

Fixed

millerf · 2020-04-29T16:19:22Z

I found another bug on wordpress.com that I will fix now:
You can click on a trashed page, and it sends you to a 404... Posts cannot be clicked when trashed.

Or is it because my site hasn't been launched yet? I am a bit confused...

kwight · 2020-04-29T18:10:08Z

You can click on a trashed page, and it sends you to a 404... Posts cannot be clicked when trashed.

Well spotted @millerf , definitely a bug: #41640

mmtr

Thanks for working on this, @millerf! Experience is much more consistent now ❤️. Didn't find any regression so let's merge this after addressing the couple of very minor comments below.

client/my-sites/post-relative-time-status/index.jsx

mmtr · 2020-04-30T09:26:24Z

It turns out it is coming from the back-end. They prefix the post title with "private: " when searching.

Nice catch! I'll file an issue so we can fix that server-side once this lands.

Co-Authored-By: Miguel Torres <miguelmariatorresrojas@gmail.com>

millerf · 2020-04-30T10:05:00Z

@mmtr I commited your suggestions, I think this can be landed safely now

millerf added 6 commits April 20, 2020 20:12

Display the right date all the time

e5611f3

Small refactor as post cannot be null (props.isRequired)

3cee3d4

clean up - includeNonDraftStatuses was never used across the project

d676b2e

Fix CamelCase

36be7c5

More refactor.

5ca266b

Added "sticky" status for posts on search and listing

Refactor label, use only one method to display time label

38dc57d

Add sticky Label for posts

matticbot added the OSS Citizen label Apr 20, 2020

Revert "Fix CamelCase"

32b2d1b

36be7c5

millerf mentioned this pull request Apr 20, 2020

Enhance/display important dates on search #41242

Closed

Minor translation fix

1434be6

a8ci18n reviewed Apr 20, 2020

View reviewed changes

client/my-sites/post-relative-time-status/index.jsx Outdated Show resolved Hide resolved

a8ci18n reviewed Apr 20, 2020

View reviewed changes

client/my-sites/post-relative-time-status/index.jsx Outdated Show resolved Hide resolved

sixhours added Posts [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Apr 20, 2020

sixhours requested a review from a team April 20, 2020 19:16

Fix date for pendingPost

aca3eb5

a8ci18n reviewed Apr 20, 2020

View reviewed changes

client/my-sites/post-relative-time-status/index.jsx Outdated Show resolved Hide resolved

mmtr reviewed Apr 21, 2020

View reviewed changes

client/my-sites/post-relative-time-status/index.jsx Outdated Show resolved Hide resolved

Fix translation for scheduled posts

3f0bb96

a8ci18n reviewed Apr 21, 2020

View reviewed changes

client/my-sites/post-relative-time-status/index.jsx Outdated Show resolved Hide resolved

a8ci18n reviewed Apr 21, 2020

View reviewed changes

client/my-sites/post-relative-time-status/index.jsx Outdated Show resolved Hide resolved

millerf added 2 commits April 21, 2020 12:18

use for scheduled posts

2ed0eed

Small refactor for readability

7be3d67

millerf added 3 commits April 27, 2020 18:27

Remove negated variable for clarity

75965c2

refactor getLabel to remove onlySticky and onlyPending parameters

20c0305

rename displayedTime vairables to make more sense

6157493

mmtr reviewed Apr 28, 2020

View reviewed changes

client/my-sites/post-relative-time-status/index.jsx Outdated Show resolved Hide resolved

millerf force-pushed the enhance/display_important_dates_on_search branch from 4d9b98e to 6157493 Compare April 28, 2020 09:03

millerf added 2 commits April 28, 2020 11:03

minor JsDocs and formatting fixes

e651d84

minor changes in variable names

db74c42

Pass private as a label

cb7b07d

Fixes Automattic#41245

fix translations

5defad5

a8ci18n reviewed Apr 28, 2020

View reviewed changes

client/my-sites/post-relative-time-status/index.jsx Show resolved Hide resolved

mmtr reviewed Apr 29, 2020

View reviewed changes

millerf added 2 commits April 29, 2020 18:15

Add forgotten case for 'private' posts date

6b0fcda

Add better comments for translators for dates

effba2a

mmtr approved these changes Apr 30, 2020

View reviewed changes

client/my-sites/post-relative-time-status/index.jsx Outdated Show resolved Hide resolved

client/my-sites/post-relative-time-status/index.jsx Outdated Show resolved Hide resolved

millerf and others added 2 commits April 30, 2020 12:03

Update client/my-sites/post-relative-time-status/index.jsx

edd82c6

Co-Authored-By: Miguel Torres <miguelmariatorresrojas@gmail.com>

Update client/my-sites/post-relative-time-status/index.jsx

0380076

Co-Authored-By: Miguel Torres <miguelmariatorresrojas@gmail.com>

mmtr merged commit ace3c71 into Automattic:master Apr 30, 2020

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 30, 2020

millerf mentioned this pull request May 1, 2020

Private posts and pages are handled as "published" #41245

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance/display important dates on search #41301

Enhance/display important dates on search #41301

millerf commented Apr 20, 2020

millerf commented Apr 20, 2020

sixhours commented Apr 20, 2020

millerf commented Apr 20, 2020

millerf commented Apr 21, 2020 •

edited

mmtr commented Apr 21, 2020

millerf commented Apr 21, 2020

mmtr commented Apr 21, 2020 •

edited

mmtr Apr 28, 2020

mmtr Apr 28, 2020

akirk commented Apr 28, 2020

mmtr commented Apr 28, 2020 •

edited

millerf commented Apr 28, 2020

akirk commented Apr 28, 2020

mmtr left a comment •

edited

mmtr Apr 29, 2020

millerf commented Apr 29, 2020 •

edited

millerf commented Apr 29, 2020

millerf commented Apr 29, 2020 •

edited

kwight commented Apr 29, 2020

mmtr left a comment

mmtr commented Apr 30, 2020

millerf commented Apr 30, 2020

Enhance/display important dates on search #41301

Enhance/display important dates on search #41301

Conversation

millerf commented Apr 20, 2020

Changes proposed in this Pull Request

millerf commented Apr 20, 2020

sixhours commented Apr 20, 2020

millerf commented Apr 20, 2020

millerf commented Apr 21, 2020 • edited

mmtr commented Apr 21, 2020

millerf commented Apr 21, 2020

mmtr commented Apr 21, 2020 • edited

mmtr Apr 28, 2020

Choose a reason for hiding this comment

mmtr Apr 28, 2020

Choose a reason for hiding this comment

akirk commented Apr 28, 2020

mmtr commented Apr 28, 2020 • edited

millerf commented Apr 28, 2020

akirk commented Apr 28, 2020

mmtr left a comment • edited

Choose a reason for hiding this comment

mmtr Apr 29, 2020

Choose a reason for hiding this comment

millerf commented Apr 29, 2020 • edited

millerf commented Apr 29, 2020

millerf commented Apr 29, 2020 • edited

kwight commented Apr 29, 2020

mmtr left a comment

Choose a reason for hiding this comment

mmtr commented Apr 30, 2020

millerf commented Apr 30, 2020

millerf commented Apr 21, 2020 •

edited

mmtr commented Apr 21, 2020 •

edited

mmtr commented Apr 28, 2020 •

edited

mmtr left a comment •

edited

millerf commented Apr 29, 2020 •

edited

millerf commented Apr 29, 2020 •

edited