A new entry preview for the backend #218
Comments
|
I was having a go at this today and was thinking: This should be easy enough, just instead of creating a preview, create a new window.open() popup with the normal frontend and show the entry data as such. But this proves as really really painful; the saving of entries is done with iframes, not only previewing, to make sure that a save does not block editing an entry and can be performed in the background. Also, if you preview, the entry may not be saved in the database, but we need to transport the data. I tried to go the short-route and make the preview iframe create a window.open() request to the blog's frontend, and wanted to fetch the sesison data to display the blog entry. But such window.open() requests in the iframe return are blocked by the browser. So a lot of users would no longer see a preview of the entry, unless told to unblock it in their browser (which, let's face it, a lot of users willnot understand). Plus, modern browsers don't allow to "focus" a popped up window that lives as a new tab, at least "preview_window.focus()" doesn't seem to work. This here is my simple diff: https://gist.github.com/garvinhicking/5aa581920139f4847d59 but I believe this will lead to nothing useful in rea-life :-( |
|
I would really like to see a “true frontend preview” here. However, since I'm not a PHP coder, I can't say whether this is technically possible in s9y. Could we have a “ghost article” in the database which can never be published and is only used to create a frontend preview of the article the user is currently editing? That “ghost article” would only be visible to logged in users, and the preview would simply open this ghost article in a new tab. (I imagine this is more complicated in a multi-user context, though.) |
|
Not possible because multiple editors could create multiple entries that await preview, yes |
|
(Plus, the challenge really is "open in a new tab". Browsers block this, very effectively) |
|
Bummer. Multiple “ghost entries” that only exist temporarily would be a performance issue, I assume? (But we can do that for published entries.) |
|
I think this is a dead-end route; we cannot save the data in the database, it must be drawn from temporary data inside the user's session data, else it would really pollute the database. Ideally, the "click" on the preview button should not execute a POST to the page itself, but open the preview window directly. But that would change a LOT of the "save entry" functionality, this would mean a near-total rewrite of the whole entry saving procedure; too complicated for me to perform, I think this will break more than it fixes. If there only was a way to properly open the popup without the browser interfering, within the iframe. :( |
|
That's why my original idea was to avoid |
|
For me the iframe sucks not because it is an iframe and on the same page like the editor. I don't really care for the new popup. The current iframe-preview sucks because we simulate the frontend partly in the preview template. The right way to do it is to let the frontend render the entry, then take that entry, and show it somehow - that could be perfectly fine in an iframe then. So, can we combine the new approach and the iframe one? Point the iframe to the frontend and use session data to render the entry there (while blocking sidebars, comment section and footer). |
|
There's no way to embed the frontend inside an iframe; which is why the preview_iframe was created in first place. This would mean that every existing frontend template would not work in a "new" iframed preview, and we would basically be building the same thing that already exists (preview_iframe.tpl IS the blog's theme content without footer and sidebars :-)) |
Why? Is there an iframe access protection I don't know? On a click on preview, we call the core. He sets the content of entry-form into SESSION variables and creates an iframe like that:
That is not what I see in https://github.com/s9y/Serendipity/blob/a2130e3d89e9423f629c0cc6b4186c178ee7e9d7/templates/2k11/preview_iframe.tpl I see that it has its own head, even modified to point to admin files. And I see custom js doing stuff. That current tpl-approach could be salvaged if we separate the entry head out of the entries.tpl and in general do nothing else there than calling frontend templates. But I don't see that being easier than just calling the frontend directly. |
|
Templates simply don't have an option to omit header / sidebars from the output, that'S what I mean with "no way to embed the frontend". We can of course always embed it FULLY, but that won't fit inside an iframe in most cases. The preview_iframe usually was meant to recreate the frontend template with the whole structure that is required for the usual CSS to render the blog entry only, without headers, sidebars and footers. In 2k11 and some other themes it got munged to print additional output in admin layout, that's where the lines began to blur. We can't really seperate entries.tpl, we always need the index.tpl part of an entry too, but then all the header etc. will be emitted. |
|
I lost track of this, but I just fell over this with 2.0.1: |
|
What's your proposal? |
|
(Note that the iframe at this point does not know if it shows a preview or "just" the saving message) |
This is why I referenced this issue from #302. See comment by @donchambers there – it is actually not only clumsy but kind of an issue because those messages might not get styled properly because they are part of the preview. I don't know if that's possible with a feasible amount of effort, but ideally, the messages for the preview should not be part of the preview so that they get styled by the backend stylesheet, if that makes any sense at all. |
Avoid the iframe at all on entry save. |
|
Have you read this entry and checked my comments on why the iframe is actually a benefit for saving entries, because it decouples the action of saving from the originating HTTP request in case errors occur and the entry would otherwise be lost? |
|
Ah I see this thread doesn't hold these comments, probably I made them on the forum or somewhere else. The save process sends pingbacks etc. and can easily stall or timeout. Which is why the iframe takes care of the saving so when an error occurs, the usual backend with the current entry would still be there, and feedback is sent directly. XML-RPC methods and trackbacks can take up anything from 1 to 60 seconds, and users might think something failed when they don't get any output. So we definitely need a method of decoupling the process, and I honestly don't think how this can easily be done without breaking a lot more things (see above). |
|
While I am probably unable to contribute to the solution from a technical perspective, I would be more than happy to test the hell out of any proposed solution! |
|
Ok, thats a point. |
|
Yeah but preview_iframe.tpl is used for both the preview and saving. It would mean we'd need a "preview_iframe.tpl" and "save_iframe.tpl" for the future maybe. save_iframe.tpl would use the backend CSS and preview_iframe.tpl the frontend CSS. However it would mean also rewriting parts of the iframe creation because the way it is now the file is rendered and prepared the same for both. I'd be happy to give feedback on possible implementations and together with @donchambers test them |
|
It should not use 2k11's preview_iframe.tpl at all if a theme provides one of its own. And it shouldn't presume any particular theme has specific ID names such as #page, #content, etc. |
No. To get an idea of what I mean (watch out for <head>
<meta charset="{$head_charset}">
<title>{$CONST.SERENDIPITY_ADMIN_SUITE}</title>
<meta name="generator" content="Serendipity v.{$serendipityVersion}">
<meta name="viewport" content="width=device-width, initial-scale=1">
{if $mode != 'save'}
{if $template_option.webfonts == 'droid'}
<link rel="stylesheet" href="//fonts.googleapis.com/css?family=Droid+Sans:400,700">
{elseif $template_option.webfonts == 'ptsans'}
<link rel="stylesheet" href="//fonts.googleapis.com/css?family=PT+Sans:400,400italic,700,700italic">
{elseif $template_option.webfonts == 'osans'}
<link rel="stylesheet" href="//fonts.googleapis.com/css?family=Open+Sans:400,400italic,700,700italic">
{elseif $template_option.webfonts == 'cabin'}
<link rel="stylesheet" href="//fonts.googleapis.com/css?family=Cabin:400,400italic,700,700italic">
{elseif $template_option.webfonts == 'ubuntu'}
<link rel="stylesheet" href="//fonts.googleapis.com/css?family=Ubuntu:400,400italic,700,700italic">
{elseif $template_option.webfonts == 'dserif'}
<link rel="stylesheet" href="//fonts.googleapis.com/css?family=Droid+Serif:400,400italic,700,700italic">
{/if}
<link rel="stylesheet" href="{$serendipityHTTPPath}{$serendipityRewritePrefix}serendipity.css">
<script src="{serendipity_getFile file="admin/js/modernizr-2.8.3.min.js"}"></script>
{/if}
{serendipity_hookPlugin hook="backend_header" hookAll="true"}
{if $mode != 'save'}
<script src="{serendipity_getFile file='admin/js/plugins.js'}"></script>
<script src="{serendipity_getFile file='admin/serendipity_editor.js'}"></script>
{else}
<link rel="stylesheet" href="{serendipity_getFile file='admin/simple_preview.css'}">
{/if}
....and so on This could get very small on output for the "save" mode. And it is very simple to implement! |
Generally yes, and could be easily implemented by adding the https://github.com/s9y/Serendipity/blob/master/include/functions_config.inc.php#L837 to both preview file cases, but not in this case IMHO, since that would mean to rewrite every themes preview file to fit the needs. It is much more easy to just make the default theme (2k11) fit the "save" mode case, like shown above. |
|
Cool! Feel free to submit the patch, we can then check it out!
|
|
Garvin, would this page need any of the js tweaks we use? To say, what else does mode "save" create than some error/success messages and some external links to the entry and the trackbacks? |
|
The one thing we/you have to look out for is the js used to set the id of the entry. Otherwise, the save mode does not use any JS I think. |
|
Oh, it uses the JS for the cache of course, the entry cache is invalidated in that box I think when saving the entry. |
|
Isn't the xml-rpc stuff and the id created before? I am only talking about modifying the tpl and I already tested the example above with a normal page (no trackbacks), which worked well without any js. |
|
I'm not sure anymore whether it is still like this, or it only was like this. But when writing a new entry, you don't have an id in the entry editor form. As soon as the entry is generated, the id is generated, and I vaguely remember there existed the approach to break out of the save iframe to set the new entry id in the entry editor form, so edits after the save will edit the entry, not create a new one. That would affect the tpl.
Yes. I don't think it is possible to avoid this, as I don't see other opportunities to execute the JS on entry save. |
|
Hmm bad... I definitely need more info to not break something. And there also is this for upcoming 2.0.2 a8b0aeb I am not sure - except for the cache buster, why Garvn did not easily use Too much to be thought of to commit a lightweight patch... |
|
It would be required to check all backend_save and backend_publish hooks in all event plugins of core and spartacus to see what is used there. Basically removing js funcs there would mean a BC break and be documented. Honestly I don't think dropping js would be good; too much work to check compatibility and possibly break stuff, and why? The js is cached so it wouldnt really impact the iframe there I think...
|
|
Still something we want to pursue? I was kind of happy with the current state the last year. |
|
As much as I would like a different way to implement the preview, it seems technically next to impossible as far as I understand the discussion. :-/ |
|
Then I'll close here for now |
This might even be “far future”. It is by no means critical or blocking for any release.
The current implementation of previewing an entry in the backend (using an
iframe) is clumsy, prone to error and likely to make developing backend and frontend themes only harder in the future. See 5ee2f4b and comments for an example. It has also historically always been hard for frontend theme authors to get thepreview_iframe.tplquite right. We've had occurrences where a mysterious empty spacerdivappeared in the backend preview etc.We should come up with an easier solution for an entry preview for the new backend, maybe just using a frontend preview?
This definitely needs discussion, which should be held in the forums. I'm just putting this here for future reference (yes, that means “so we don't forget about it”).
The text was updated successfully, but these errors were encountered: