inconsistencies in which layout is used for the preview tab

Bug #100947 reported by Joachim Scmitz
10
Affects Status Importance Assigned to Milestone
Silva
Fix Released
Medium
Kit Blake

Bug Description

If one has his own layout installed, the preview_tab still shows the default
layout, one has to click on presentation preview to see the installed layout.
It would be nice to see the preview in the own layout.

Tags: silva-future
Revision history for this message
Martijn Faassen (faassen) wrote :

Showing the preview in its own layout has been a wish for Silva for a long time.
We won't manage this in 0.9.3 yet though.

Revision history for this message
Andy Altepeter (aaltepet) wrote :

Kit, is this still a wish? And/or i sit something you'd like me to play around with? I've always thought of the preview tab as a "pre-preview", something you look at to see if, in general, your content is setup right. I don't find it terribly useful. Now that we have the "public preview" middlebar link on the edit tab of most content, perhaps it isn't even a wish anymore.

I can think of certain objects where there is no real issue with seeing the real public preview; some custom objects I wrote work well with the preview tab, as the public preview is more "heavy", with other content (pulled in from the layout) and stuff.

Changed in silva:
assignee: nobody → kitblake
importance: Low → Wishlist
status: Confirmed → Incomplete
Revision history for this message
Kit Blake (kitblake) wrote :

Andy, I agree, I don't find the pre-preview useful either, and I'd really like it to display the public preview. Especially since we now show (the same) pre-preview in the edit screen of published Silva docs (developers need to adjust their content types to do this).

But the public preview is an entire html page; we can't 'nest' it within the Silva UI. We did spend some time once trying to make it work with an iframe, but you must 'size' an iframe in pixels, you can't say it's 100% high. Now I think we should reverse the idea, and instead of iframing the preview, we should iframe the Silva navigation (breadrumb and tabs).

If this words it'll make the preview process much simpler. Also it'll make the Silva screens simpler, always a good thing. So yes, give it a shot (is the above clear?).

Changed in silva:
assignee: kitblake → aaltepet
Changed in silva:
status: Incomplete → Confirmed
Kit Blake (kitblake)
Changed in silva:
milestone: none → 2.1
Revision history for this message
Kit Blake (kitblake) wrote :

First some explanation: what I would like is that, when a user goes to the preview tab, he sees what is now public preview (preview_html). But, instead of the superimposed publish and back buttons, I want the 'normal' Silva breadcrumb and tabs to be at the top, exactly the same as tab_preview today.

This is tricky because we're loading all the customer's stylesheets and layout, but we also need the Silva css/layout for the matrix nav. Either we make a special Silva css for just this preview case, that has duplicated selectors for the breadcrumb and tabs, with odd names that won't conflict with a customer's styles, or we load two separate html pages, using an iframe. This seems to be the easier approach since we won't have to worry about conflicting.

Following is a way to get that iframe showing up. If you delete the code in www/preview_buttons.zpt and put this in, then go to a versioned content public preview, you'll see what I mean:

<style rel="stylesheet" type="text/css">
body {
position:relative;
top:50px;
}
div.matrixnav {
margin-top: -72px;
}
</style>

<div class="matrixnav">
<iframe width="100%"
  height="50px"
  marginheight="0"
  marginwidth="0"
  frameborder="0"
  scrolling="no"
  tal:attributes="src string:${here/absolute_url}/edit/tab_preview">
</iframe>

Since the iframe is invisible the user has no idea it's there and everything looks like the normal preview screen except the content will be in the public layout.

Some points about the code:
+ The body of the previewed page is shifted down 50 pixels to make room for the iframe containing the Silva matrix nav.
+ The div that contains the iframe has a negative top margin to compensate (I'm not sure why the number isn't the same, but this works in FF, and we'll make it work in IE7).
+ The matrix nav doesn't function properly because it's in an iframe. We'll have to set all the link targets to go to _parent. We can just do that because in all other cases there is no parent frame.
+ We won't need the "public preview" button :)
+ We should keep the middleground with the "publish now" button because there's an edge case where a user is very deep in a site and the breadcrumb trail gets so long it breaks onto two lines. The button disappears but they can live with that and go to one of the other screens to publish.

Functional aspects:
+ This will have to be implemented for previews of containers too. Right now this (www/preview_buttons.zpt) only works for versioned content.
+ We should make a "publish now" button for container previews that publishes the index.

Ideas, critique?

Changed in silva:
importance: Wishlist → Medium
Revision history for this message
Eric Casteleijn (thisfred) wrote :

So does this render the content without the public navigation? If not, wouldn't that be a good idea? I think it should be possible, if we make a bare layout template that only loads the public css and renders only the content. Probably we could even get that working without having to resort to an iframe.

Revision history for this message
Kit Blake (kitblake) wrote :

Jasper and I had some extensive train time yesterday on the way to a client, and we've come to a conclusion (feel free to quibble :)). The iframe solution looks good but has a couple of dirty tricks. One of them is loading a style tag within the body of a page, actually within the Silva content. The other is shifting the body down 50 pixels to make room for the iframe is dubious.

Jasper's suggestion was to use a frameset, and after discussing the consequences we think it's the cleanest way to go. This would mean that tab_previww.pt would contain frameset code with a condition that checks if the meta_type is a container or versioned content. If not, the preview remains the same, e.g. preview for an image doesn't need a frameset. If the frameset does come into play, it means the two documents cannot possibly conflict with each other.

The frameset would have two rows:
<frameset rows="50px,*">

and have two frame sources:
1. A page template that renders the breadcrumbs, tabs, and middleground (adjusted with target="_parent" links), exactly as tab_preview looks today.
2. The preview_html.

We can throw away all the hacky code that inserts the publisher buttons in preview_html in VersionedContent.py. This also makes it easier to preview containers. We'll still have to introspect and see if the container's index can be published and show or not show the publish button in the middleground.

Jasper thinks a frameset sounds yucky but it's needed, and thisfred said he thinks its a justified use case. Andy, any thoughts? Does this make sense?

Revision history for this message
Kit Blake (kitblake) wrote :

BTW, I got this (the iframe solution) working with Silva Layout too. Jasper thinks we should come up with some solution where, when creating a new skin, it's not necessary for a developer to register the preview view. Probably most developers don't think of this (until they get an error in the preview screen). Then after some digging they figure out they need to register that view. It would be better if the preview just previewed, somehow.

Revision history for this message
Andy Altepeter (aaltepet) wrote :

Regarding the frameset solution: Kit has laid it out pretty clearly, and it does make sense. I do wonder, though (please confirm) whether an absolute 50 pixels is the best solution. It could be possible that the client has increased the font size, pushing the top bars down a bit more. Or perhaps there is a long breadcrumb. I thought Kit removed the fixed height on something up there.

It should be possible to inspect the height of the content in the top frame, and adjust the height of the frame if there is more that 50 px of content. The frame could still start out at 50px. This should be possible on page load; there may be a slight flicker, but at least there wouldn't be a scrollbar.

I think Kit's last comment about custom SilvaLayouts needing to redefine the preview skin should be a separate issue. W/out knowing the internals, I could imagine the preview view "just previewing" by inspecting the skin set on the content/model, and switching to that(?)

I haven't felt well enough at night this week to do some good Silva coding...so hopefully tomorrow (Thursday) will be better.

Revision history for this message
Kit Blake (kitblake) wrote :

Inspecting the height of the content is a good idea, because we don't know how high it could be, only in a default Silva. Long titles making long breadcrumb trails can push it down, or font size. Fortunately the template will only contain those elements so it should be measurable.

For the SilvaLayout preview I'll make a separate issue. I don't grok it completely either so I'll assign it to Jasper :)

Revision history for this message
Andy Altepeter (aaltepet) wrote :

I've created a branch where this work is being done:
https://svn.infrae.com/Silva/branch/aaltepet/Silva-preview-tab-100947

Revision history for this message
Andy Altepeter (aaltepet) wrote :

Things that work in this branch:
1) automatic resizing of the top frame if the content is large. (try running zope in development mode, and you get the ZMI links in the top frame)
2) Containers show the "publish now" button if the index document is not published
3) links in the top frame go to _parent. The <base> tag has a "target" attribute that sets up the default target for links in a document!

Things that don't work:
1) logic to choose standard/legacy preview mode (e.g. preview tab of images / files / etc, but they're not all assets?)
2) The publish now button. (the new tab_preview isn't accepting the message type and feedback yet...likely these will be passed in the frame src as parameters to the top frame)
3) the preview tab of content...but containers do work.

Question:
1) Should this have a sidebar in the frame-based? We're looking that functionality
2) Should this have the SMI footer?

Revision history for this message
Kit Blake (kitblake) wrote :

Great that the base tag works. You're correct, there are also Content objects (AutoTOC and Indexer) and they will need to preview with the frameset too. So we check for Content, VersionedContent or Containers. All the others are assets.

1. No sidebar. Many sites need the full width of the screen. I think it's best that the user sees the layout as it will appear if the public page is loaded.
2. We can skip the SMI footer too. That would require another frame and the functionality in there is available in every other screen.

Revision history for this message
Kit Blake (kitblake) wrote :

The Silva Link is an odd case. When a user goes to preview, they end up in the destination of the link. I've never liked this about the Link object, as there's no way to see what it's (versioned) content is, you have to make a new version to see. I think its preview should show a rendered http link, much like the preview of a file displays a link to a file. This is a separate issue, but I'm mentioning it in case it seems like the frameset is misbehaving :)

Revision history for this message
Andy Altepeter (aaltepet) wrote :

The things I reported that didn't work in https://bugs.launchpad.net/silva/+bug/100947/comments/11
Are now working. I've also fixed a few more issues I found while testing. Two things have questions about:

1) Should there be a decoration (border) between the top frame and the previewed content?
2) Should the frameset be periodically checking the top frame height and resize if needed? Use case: go to preview tab, then make window smaller -- the tabs will begin to wrap...that's an extreme example, but if the breadcrumb is almost long enough to wrap, and then the person resizes the window for some reason, e.g. to check a mockup in photoshop or some other thing related to their workflow. This wouldn't be difficult to add.

Note: I haven't tested this yet with SilvaLayout

Revision history for this message
Kit Blake (kitblake) wrote :

For #1 I'd say no, not if it's a frameset border like the olde days. We can always add a border with css.
#2, if it's really not difficult, but it's such an edge case with an easy workaround (reload :) I'd say skip it.

Revision history for this message
Andy Altepeter (aaltepet) wrote :

Regarding #1, I just wanted to raise the point, and leave it to you to decide whether to have a border / divider between the two, and also how to style it. Sure, you could put a css border on the frame, or you could style the frame border with css.

I think at this point, this feature seems to be working as specified, so I'd like to open this to others for testing. Or, if you just want me to merge this in with the trunk I could do that, too.

Revision history for this message
Andy Altepeter (aaltepet) wrote :

OK, the branch I was working on has been merged with the trunk.

Changed in silva:
assignee: aaltepet → kitblake
Revision history for this message
Andy Altepeter (aaltepet) wrote :

the color of the top frame isn't changing depending on whether model is a container or content :-(

Changed in silva:
assignee: kitblake → aaltepet
Revision history for this message
Andy Altepeter (aaltepet) wrote :

also, the sidebar in the preview tab of assets links to 'tab_preview_noframes' for the objects, rather than 'tab_preview'.

Revision history for this message
Andy Altepeter (aaltepet) wrote :

The color issue is fixed now.

The sidebar is also fixed. It doesn't use the sidebar cache (which apparently _only_ works with containers). The sidebar cache uses a variable (vein, tab_name) to compute the url, while the standard sibling sidebar uses the variable "template_id", or the templates id if template_id is not defined.

So, it seems the variables and usage differs depending on whether the sidebar cache is being used. The sidebar cache I think is a newer component, and was implemented with slightly different usage spec.

Changed in silva:
status: Confirmed → Fix Committed
Revision history for this message
Kit Blake (kitblake) wrote :

I've adjusted the presentation: the preview top frame displays the middleground, which may have a publish now button, unless there is feedback, which only happens when there's not a publish now button. I also moved the 'closed notice' from the old preview to the top frame. Nice work, this looks really good :) and it's a big improvement in Silva usability.

Still have to remove all the public preview buttons, but I fear that will break tests so I'll wait for testmaster Todd on that.

In answer to your comment on the cache, it would be best if they rendered the same, but since the content level sidebar only wakes up items in the current container caching is not so necessary as the container sidebar, which can wake up an entire branch.

Revision history for this message
Kit Blake (kitblake) wrote :

Andy, I've found one glitch in that when a document with a toc is previewed, the links in the toc go to ".../edit/tab_preview" and don't have a target. So the frameset nests itself. :( Since links in the content and any links in the public template such as navigation won't have a target, maybe we should just remove the /edit/tab_preview. You probably already know this from your toc work, but it happens in:
/SilvaDocument/widgets/element/doc_elements/toc/render_tree.py
and
SilvaDocument/widgets/element/doc_elements/toc/mode_preview/render.py
What do you think? If you agree, could you remove the code?

Revision history for this message
Andy Altepeter (aaltepet) wrote :

Kit - what you say makes sense for now. I've removed the offending append_to_url. I've identified at least 4 issues dealing with revising link rendering to better support public vs. preview links, and I think this TOC element issue can be addressed at that time. I'll add a note about TOCs here:
https://bugs.launchpad.net/silva/+bug/101364

Changed in silva:
assignee: aaltepet → kitblake
Changed in silva:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.