Rework preview mechanism based on StackedContent implementation #105
Conversation
|
@kows Thank you for the pull request. This has been on my to-do list for a long time, so good to see it being tackled. I'll schedule in a time to review it, (I've got some other DTGE PRs that I need to review, I'll bundle them all together 👍) |
To bring inline with our previous code formatting.
This follows our approach in Stacked Content and gives the intent it for the Page, rather than a "node", (which becomes confusing in this modern NC/DTGE/StackedContent world) Added an additional check that the page ID is greater than 0. A newly created page has an ID of -1, which means that it isn't in the cache or database and would throw an exception.
leekelleher
force-pushed
the
feature/preview-unpublished
branch
from
71acbca to
a18179b
Compare
| page = new UnpublishedContent(nodeId, Services); | ||
| } | ||
|
|
||
| var culture = UmbracoContext.Application.Services.ContentService.GetById(nodeId).GetCulture(); |
There was a problem hiding this comment.
I'm curious about this call. Wondering if there's a lighter way to get the culture for a specific node?
There was a problem hiding this comment.
Wondering as well, but since it's unpublished I guessed DB was the only way?
|
|
||
| var culture = UmbracoContext.Application.Services.ContentService.GetById(nodeId).GetCulture(); | ||
| System.Threading.Thread.CurrentThread.CurrentCulture = culture; | ||
| System.Threading.Thread.CurrentThread.CurrentUICulture = culture; |
There was a problem hiding this comment.
We'll need to clarify that this does only set the culture for the current thread, and not the application.
@mattbrailsford Do you have any insight on this?
Also, do we think this should also be in Stacked Content's previewer? (Although no one has mentioned culture issues to us).
| { | ||
| public IPublishedContent Page { get; set; } | ||
|
|
||
| public FormDataCollection Values { get; set; } |
There was a problem hiding this comment.
I'm not sure about exposing FormDataCollection from the preview model. It's not a showstopper, but thinking that we already know what the properties are, (e.g. "editorAlias", "contentTypeAlias", etc), then we could strongly-type them? (since we're already introducing a new view-model for the previewer.
Like I say, it's not a showstopper for this PR ... just something for me & @mattbrailsford to discuss.
| getEditorMarkupForDocTypePartial: function (nodeId, id, editorAlias, contentTypeAlias, value, viewPath, previewViewPath, published) { | ||
| var url = umbRequestHelper.convertVirtualToAbsolutePath("~/" + (published ? nodeId : "") + "?dtgePreview=1" + (published ? "" : "&nodeId=" + nodeId)); | ||
| getEditorMarkupForDocTypePartial: function (nodeId, id, editorAlias, contentTypeAlias, value, viewPath, previewViewPath) { | ||
| var url = umbRequestHelper.convertVirtualToAbsolutePath("~/umbraco/backoffice/DocTypeGridEditorApi/DocTypeGridEditorApi/GetPreviewMarkup?dtgePreview=1&nodeId=" + nodeId); |
There was a problem hiding this comment.
Do we still need the "dtgePreview" querystring?
There are other parts of the codebase that check for it, but I think they are related to the old preview mechanism.
|
@kows Thank you for the PR. I'm going to merge it in to a temporary branch for now. It's definitely the feature we want, but I want to discuss a few things with @mattbrailsford before merging in to the |
|
I'm not sure if you rather have me log this in a separate issue (just let me know). We were trying out the 0.6.0 beta today to see if the fix for unpublished content previews works. When in the new preview mode the |
|
@tomvanenckevort If you open a separate ticket for this, that'd be great, thanks. |
#59
Had a go at this and the basics seem to be working so far.
PR'ing to get the discussion going :)
Didn't see anything for that in StackedContent.