Skip to content
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

Setting a featured image and clicking Preview does not show the featured image #9151

Closed
noisysocks opened this issue Aug 20, 2018 · 17 comments · Fixed by #12097
Closed

Setting a featured image and clicking Preview does not show the featured image #9151

noisysocks opened this issue Aug 20, 2018 · 17 comments · Fixed by #12097

Comments

@noisysocks
Copy link
Member

@noisysocks noisysocks commented Aug 20, 2018

Describe the bug
After you set a feature image, clicking Preview does not show the feature image on the frontend. This is a very confusing experience.

To Reproduce

  1. Create a new post
  2. Insert a title or add some text
  3. Using the Document sidebar, set a Featured Image
  4. Click Preview in the top right

Expected behavior
The selected featured image should be visible at the top of the post, but it is not.

Screenshots
featured-image

Additional context
Encountered this during user testing. Clicking Save Draft before Preview works as expected.

@noisysocks

This comment has been minimized.

Copy link
Member Author

@noisysocks noisysocks commented Aug 20, 2018

Is this because the autosave endpoint does not accept the featured_media field? cc. @azaozz @adamsilverstein @danielbachhuber

@danielbachhuber

This comment has been minimized.

Copy link
Member

@danielbachhuber danielbachhuber commented Aug 20, 2018

The underlying issue is that the Classic Editor saves all post meta during a Preview request. However, Gutenberg only saves a few specific fields.

I think https://core.trac.wordpress.org/ticket/20299 is defacto Trac ticket on the subject.

I'm not sure there's an "easy fix" for this one. We have some liberty on how we can address the issue though. I don't have strong opinions on the preferred implementation at this point.

@youknowriad

This comment has been minimized.

Copy link
Contributor

@youknowriad youknowriad commented Aug 20, 2018

Also like suggested here #7767 (comment) The fact that the classic edditor savese those metas is considered a bug, because they are not saved as a "revision" but they really update the post object while the intent is to only preview and not save the changes.

I closed the other issue as Invalid, I think we should dod the same.

@designsimply

This comment has been minimized.

Copy link
Member

@designsimply designsimply commented Aug 20, 2018

Are you suggesting to close the issue as invalid because you don't think the ability to preview changes to featured images should be doable or because the problem is technically happening due to an upstream bug in the way that autosaves work?

@youknowriad

This comment has been minimized.

Copy link
Contributor

@youknowriad youknowriad commented Aug 20, 2018

Because I don't think it's doable without a very big refactoring to how revisions work in WordPress and the fact that it works in the classic editor is just a consequence of a bug in the classic editor.

@danielbachhuber

This comment has been minimized.

Copy link
Member

@danielbachhuber danielbachhuber commented Aug 20, 2018

We could potentially save all post meta only to the autosave object, which would mitigate @azaozz's original concerns about bloating revisions with post meta (and why revisions meta, to my knowledge, wasn't ever implemented).

@danielbachhuber

This comment has been minimized.

Copy link
Member

@danielbachhuber danielbachhuber commented Aug 20, 2018

We could potentially save all post meta only to the autosave object

Notably, if we do this, we'd need to make sure the UX around restoring from an autosave is updated accordingly. Either the entire autosaved object, including meta, is restored, or only the title, content, and excerpt are restored from the autosave object, even though meta is present too.

@noisysocks

This comment has been minimized.

Copy link
Member Author

@noisysocks noisysocks commented Aug 21, 2018

I don't think we should close the issue—it came up during user testing and was a considerable roadblock for them using Gutenberg.

In the Classic Editor if I set a featured image and then click Preview Changes it correctly shows a preview of the post with a featured image and does not update the published post. It looks like it does this by sending _thumbnail_id as a query variable to the preview screen, which is maybe an interim solution that we should borrow for Gutenberg.

@azaozz

This comment has been minimized.

Copy link
Contributor

@azaozz azaozz commented Aug 21, 2018

The fact that the classic edditor savese those metas is considered a bug, because they are not saved as a "revision" but they really update the post object while the intent is to only preview and not save the changes.

It's not as simple :)

The post-meta is "additional data" about the post. Whether the post is a draft, or published, or waiting for approval, etc. is just a state. The post "exists" and the meta is valid regardless of that state. In that terms the post meta "inherits" the state of the post but is not directly dependent on it (I mean, drafts can have the same meta as published posts, etc.).

The need for "post meta revisions" arose after some user input was being saved in the meta and not in the post object (mostly by plugins). I understand this is a convenient way to save post related user input, that is later used to modify the post in some way. Whether it is the best way..? :)

The old Edit Post screen saves some "meta-boxes" right away (AJAX) while others are saved when the form is submitted. This is not-the-best-way-to-do-things, but seems sufficient (as long as the plugins authors are aware of it and choose the right way for saving of the user input they need).

In Gutenberg post-meta is not saved "right away", but only when the user clicks the Save button. This seems better than on the old Edit Post screen, however previews would have to be handled differently: the user will not be able to preview a draft post unless they save it first.

Of course we could "auto-save" all of the post including the meta on clicking the Preview button and then show the actual preview. This would work well for drafts, but brings problems for published posts as the post meta would go "live" while the post content is being previewed. Fort that reason in the classic editor there are two different preview modes:

  • For drafts all data is saved and then the preview is shown.
  • Fort published posts (and scheduled, etc.) an autosave revision is created and then the existing (old) post is shown but the content of that revision is used to replace/override only title, content, excerpt. This "mostly" work well but is missing any changes/updates to the post meta (we would need to save meta revisions and match a particular meta revision with particular post revision. Not impossible, but hasn't been implemented yet.).

The tl;dr: for draft posts we would need to do "save draft" before showing the preview, i.e. save all post data including from "metaboxes".

@youknowriad

This comment has been minimized.

Copy link
Contributor

@youknowriad youknowriad commented Aug 21, 2018

The tl;dr: for draft posts we would need to do "save draft" before showing the preview, i.e. save all post data including from "metaboxes".

I know this was the case at some point, so I'm wondering if there's a valid reason for its change to "auto-save". cc @aduth

@aduth

This comment has been minimized.

Copy link
Member

@aduth aduth commented Aug 21, 2018

Previously: #7130

The client wants to create an autosave, it shouldn't care who's authored the post, whether it's a draft. This should be on the server to handle. Otherwise what purpose is the endpoint serving? We can send the entire payload, but if we're saying the prerequisite for a preview is an autosave, the incidentals should be contained within the handling of the autosave task on the server.

@danielbachhuber

This comment has been minimized.

Copy link
Member

@danielbachhuber danielbachhuber commented Oct 31, 2018

Given the timeline for shipping 5.0, it's unlikely this particular bug will be fixed in this release. Reassigning to WP 5.1 Onwards to pick up in the future.

@noisysocks

This comment has been minimized.

Copy link
Member Author

@noisysocks noisysocks commented Oct 31, 2018

Thoughts on borrowing what the Classic Editor does as an interim solution for 5.0?

In the Classic Editor if I set a featured image and then click Preview Changes it correctly shows a preview of the post with a featured image and does not update the published post. It looks like it does this by sending _thumbnail_id as a query variable to the preview screen, which is maybe an interim solution that we should borrow for Gutenberg.

@noisysocks

This comment has been minimized.

Copy link
Member Author

@noisysocks noisysocks commented Oct 31, 2018

Hm, damn. I tried it but it only seems to fix the case where you're previewing changes made to a published post. I think we also would need to change the Preview button so that it does a full save when the post is a draft, as @azaozz mentioned in #9151 (comment).

@danielbachhuber

This comment has been minimized.

Copy link
Member

@danielbachhuber danielbachhuber commented Oct 31, 2018

Yeah, it looks like this is more severe (#11280) and will need to be addressed in some form.

@adamsilverstein

This comment has been minimized.

Copy link
Contributor

@adamsilverstein adamsilverstein commented Nov 2, 2018

Noting that in my testing, this is not fixed by #11409 which saves metabox meta before previewing. I think we can make passing the thumbnail id work, this looks like a good start: https://github.com/WordPress/gutenberg/compare/try/feature-image-preview

@adamsilverstein

This comment has been minimized.

Copy link
Contributor

@adamsilverstein adamsilverstein commented Nov 17, 2018

@noisysocks your try branch looks good and solves the issue for published posts - to me this seems worth merging as is. We can resolve the issue with draft posts separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.