-
Notifications
You must be signed in to change notification settings - Fork 80
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
Fix unexpected preview crash with adding preview condition #656
Conversation
Admin/ArticleAdmin.php
Outdated
@@ -260,6 +260,8 @@ function(Localization $localization) { | |||
]; | |||
} | |||
|
|||
$previewCondition = 'route != null'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexander-schranz @wachterjohannes that was the best solution I was able to find for the preview condition 😅 it is working but somehow feels a bit wrong, do you have any better suggestion?
a79ecee
to
ec269aa
Compare
Admin/ArticleAdmin.php
Outdated
@@ -260,6 +260,7 @@ function(Localization $localization) { | |||
]; | |||
} | |||
|
|||
$previewCondition = "locale in availableLocales"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wachterjohannes Checking the locale is only possible, when we also update the PreviewForm.js in sulu/sulu like this:
- const enablePreview = !previewCondition || jexl.evalSync(previewCondition, this.resourceFormStore.data);
+ const previewData = {
+ ...this.props.router.attributes,
+ ...toJS(this.resourceFormStore.data)
+ };
+ const enablePreview = !previewCondition || jexl.evalSync(previewCondition, previewData);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there not a locale
and originalLocale
returned by the JSON response?
When we add router attributes to the conditoin we maybe should prefix it with __
like we did it with __parent
or __webspaces
already. So maybe __routeAttributes
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ec269aa
to
382fc39
Compare
382fc39
to
fa9b82f
Compare
What's in this PR?
Fixes the preview when the locale is switched to a non existing one in the article detail view.