New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Page says post updated when it's a page #3315

Closed
karmatosed opened this Issue Nov 2, 2017 · 13 comments

Comments

@karmatosed
Member

karmatosed commented Nov 2, 2017

We should change this message depending on if a page or a post:

2017-11-02 at 17 17

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Nov 2, 2017

Member

Related filter: https://developer.wordpress.org/reference/hooks/post_updated_messages/

I do not expect that this is exposed anywhere in the REST API, but could be bootstrapped for page load.

This would depend upon whether we want to respect post updated messages for custom post types specifically, since core already hard-codes labels for posts, pages, and media:

https://github.com/WordPress/WordPress/blob/7deb33b3869d12917c59bc8165b598d63e072df3/wp-admin/edit-form-advanced.php#L146-L174

Member

aduth commented Nov 2, 2017

Related filter: https://developer.wordpress.org/reference/hooks/post_updated_messages/

I do not expect that this is exposed anywhere in the REST API, but could be bootstrapped for page load.

This would depend upon whether we want to respect post updated messages for custom post types specifically, since core already hard-codes labels for posts, pages, and media:

https://github.com/WordPress/WordPress/blob/7deb33b3869d12917c59bc8165b598d63e072df3/wp-admin/edit-form-advanced.php#L146-L174

@lukecav

This comment has been minimized.

Show comment
Hide comment
@lukecav

lukecav Nov 4, 2017

Should be similar on post types, for example product.

Product published. View Product

lukecav commented Nov 4, 2017

Should be similar on post types, for example product.

Product published. View Product

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth May 14, 2018

Member

I thought I'd raised it at one point before, but since I'm not seeing it here:

We should consider making this an optional label for post types, using it within the editor UI. It seems suitable as the purpose for the labels property of a custom post type.

Member

aduth commented May 14, 2018

I thought I'd raised it at one point before, but since I'm not seeing it here:

We should consider making this an optional label for post types, using it within the editor UI. It seems suitable as the purpose for the labels property of a custom post type.

@danielbachhuber danielbachhuber self-assigned this May 25, 2018

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Jul 27, 2018

Member

Related filter: https://developer.wordpress.org/reference/hooks/post_updated_messages/

I do not expect that this is exposed anywhere in the REST API, but could be bootstrapped for page load.

Specific to post_updated_messages, we can't necessarily use this filter in Gutenberg because:

  1. The message can changed based on post state.
  2. Gutenberg can change the post's state, which would make the post_updated_messages stale.
  3. The existing post_updated_messages strings don't match what we're using in Gutenberg.

This would point towards introduce new labels on the post type object. Labels on the post type object are stateless, which mitigate the concern about modification. Also worth noting, we can't do sprintf( __( '%s updated!' ), postType ) because we need to accommodate different grammar (?) structures in each locale.

Based on a quick look, it appears we'd need to introduce at least these strings as new post type labels:

  • 'Post reverted to draft.'
  • 'Post published!'
  • 'Post published privately!'
  • 'Post scheduled!'
  • 'Post updated!'
  • 'View post'

@swissspidy @ocean90 Because I know you're relatively close to localization, is there anything you'd add or change to my assessment?

Member

danielbachhuber commented Jul 27, 2018

Related filter: https://developer.wordpress.org/reference/hooks/post_updated_messages/

I do not expect that this is exposed anywhere in the REST API, but could be bootstrapped for page load.

Specific to post_updated_messages, we can't necessarily use this filter in Gutenberg because:

  1. The message can changed based on post state.
  2. Gutenberg can change the post's state, which would make the post_updated_messages stale.
  3. The existing post_updated_messages strings don't match what we're using in Gutenberg.

This would point towards introduce new labels on the post type object. Labels on the post type object are stateless, which mitigate the concern about modification. Also worth noting, we can't do sprintf( __( '%s updated!' ), postType ) because we need to accommodate different grammar (?) structures in each locale.

Based on a quick look, it appears we'd need to introduce at least these strings as new post type labels:

  • 'Post reverted to draft.'
  • 'Post published!'
  • 'Post published privately!'
  • 'Post scheduled!'
  • 'Post updated!'
  • 'View post'

@swissspidy @ocean90 Because I know you're relatively close to localization, is there anything you'd add or change to my assessment?

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
Member

danielbachhuber commented Jul 27, 2018

@danielbachhuber danielbachhuber removed their assignment Jul 27, 2018

@swissspidy

This comment has been minimized.

Show comment
Hide comment
@swissspidy

swissspidy Jul 27, 2018

Member

Post type labels make sense, yeah.

Looking at https://core.trac.wordpress.org/browser/tags/4.9/src/wp-admin/edit-form-advanced.php#L146, I think these are needed too:

  • 'Post submitted' (for reviews)
  • 'Post restored to revision from %s.'

Also, should the 'Post scheduled' label contain the date like in the current editor, e.g. 'Post scheduled for: %s.'?

On a side note, I'm not sure how I feel about the exclamation mark in the Gutenberg strings vs. the dot in the current strings in core.

Member

swissspidy commented Jul 27, 2018

Post type labels make sense, yeah.

Looking at https://core.trac.wordpress.org/browser/tags/4.9/src/wp-admin/edit-form-advanced.php#L146, I think these are needed too:

  • 'Post submitted' (for reviews)
  • 'Post restored to revision from %s.'

Also, should the 'Post scheduled' label contain the date like in the current editor, e.g. 'Post scheduled for: %s.'?

On a side note, I'm not sure how I feel about the exclamation mark in the Gutenberg strings vs. the dot in the current strings in core.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Jul 27, 2018

Member

Flagging Needs Copy Review to get finalized strings before we create a Core patch for this.

Member

danielbachhuber commented Jul 27, 2018

Flagging Needs Copy Review to get finalized strings before we create a Core patch for this.

@kristastevens

This comment has been minimized.

Show comment
Hide comment
@kristastevens

kristastevens Jul 27, 2018

Allo allo! I saw the "needs copy review" bat signal. I think these sound fine and I agree with @swissspidy that we probably don't need exclamation points and should remove them for consistency in core. So that would leave us the following. (I added one minor revision.)

  • 'Post reverted to draft.'
  • 'Post published.'
  • 'Post published privately.'
  • 'Post scheduled.'
  • 'Post updated.'
  • 'View post.'
  • 'Post submitted for review.'
  • 'Post restored to revision from %s.'

It would be nice to include the time and date when a user schedules a post as a courtesy to the user.

  • 'Post scheduled for TIME on DATE.'

kristastevens commented Jul 27, 2018

Allo allo! I saw the "needs copy review" bat signal. I think these sound fine and I agree with @swissspidy that we probably don't need exclamation points and should remove them for consistency in core. So that would leave us the following. (I added one minor revision.)

  • 'Post reverted to draft.'
  • 'Post published.'
  • 'Post published privately.'
  • 'Post scheduled.'
  • 'Post updated.'
  • 'View post.'
  • 'Post submitted for review.'
  • 'Post restored to revision from %s.'

It would be nice to include the time and date when a user schedules a post as a courtesy to the user.

  • 'Post scheduled for TIME on DATE.'
@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Oct 4, 2018

Member

@0aveRyan said he'd pick this up today in Slack.

Member

danielbachhuber commented Oct 4, 2018

@0aveRyan said he'd pick this up today in Slack.

@nielslange

This comment has been minimized.

Show comment
Hide comment
@nielslange

nielslange Oct 5, 2018

Contributor

@0aveRyan I just saw @danielbachhuber's comment above. I've started working on this issue yesterday already and created the PR #10361 earlier today. As this is my second PR to Gutenberg and my very first JS-based PR it might be a bit bumpy though. Feel free to give me feedback on how to improve #10361 or create your own PR, if you think my implementation is not aligned with the overall Gutenberg development. 😉

Contributor

nielslange commented Oct 5, 2018

@0aveRyan I just saw @danielbachhuber's comment above. I've started working on this issue yesterday already and created the PR #10361 earlier today. As this is my second PR to Gutenberg and my very first JS-based PR it might be a bit bumpy though. Feel free to give me feedback on how to improve #10361 or create your own PR, if you think my implementation is not aligned with the overall Gutenberg development. 😉

@0aveRyan

This comment has been minimized.

Show comment
Hide comment
@0aveRyan

0aveRyan Oct 10, 2018

Member

@nielslange no worries! I just sat down to write a patch, but glad I checked back first. Jumping discussion over to that PR.

Member

0aveRyan commented Oct 10, 2018

@nielslange no worries! I just sat down to write a patch, but glad I checked back first. Jumping discussion over to that PR.

@kadamwhite

This comment has been minimized.

Show comment
Hide comment
@kadamwhite

kadamwhite Oct 16, 2018

Contributor

In the thread on PR #10361, we discussed an alternative solution which exposes label data through PHP (by adding the computed labels for each type as a labels property on the wp/v2/types endpoint) then consuming that label data in Gutenberg to construct the strings using the singular label for the specified type. Based on conversation in the #core-restapi channel, this feels like a more robust way forward than a JS-only solution. @0aveRyan do you still have any bandwidth to pick this up again?

Contributor

kadamwhite commented Oct 16, 2018

In the thread on PR #10361, we discussed an alternative solution which exposes label data through PHP (by adding the computed labels for each type as a labels property on the wp/v2/types endpoint) then consuming that label data in Gutenberg to construct the strings using the singular label for the specified type. Based on conversation in the #core-restapi channel, this feels like a more robust way forward than a JS-only solution. @0aveRyan do you still have any bandwidth to pick this up again?

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Oct 16, 2018

Member

we discussed an alternative solution which exposes label data through PHP (by adding the computed labels for each type as a labels property on the wp/v2/types endpoint) then consuming that label data in Gutenberg to construct the strings using the singular label for the specified type.

@kadamwhite Just to be clear, isn't this what I originally proposed in #3315 (comment) ?

Member

danielbachhuber commented Oct 16, 2018

we discussed an alternative solution which exposes label data through PHP (by adding the computed labels for each type as a labels property on the wp/v2/types endpoint) then consuming that label data in Gutenberg to construct the strings using the singular label for the specified type.

@kadamwhite Just to be clear, isn't this what I originally proposed in #3315 (comment) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment