-
Notifications
You must be signed in to change notification settings - Fork 383
Add content types to notices #5072
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
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -30,8 +30,10 @@ | |||||
|
|
||||||
| namespace VuFind\View\Helper\Root; | ||||||
|
|
||||||
| use Laminas\View\Helper\AbstractHelper; | ||||||
| use Laminas\View\Exception\InvalidArgumentException; | ||||||
| use Laminas\View\Helper\EscapeHtml; | ||||||
| use VuFind\ContentBlock\TemplateBased; | ||||||
| use VuFind\ServiceManager\Factory\Autowire; | ||||||
|
|
||||||
| /** | ||||||
| * Content View Helper to resolve translated pages. | ||||||
|
|
@@ -43,44 +45,46 @@ | |||||
| * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License | ||||||
| * @link https://vufind.org/wiki/development Wiki | ||||||
| */ | ||||||
| class Content extends AbstractHelper | ||||||
| class Content | ||||||
| { | ||||||
| /** | ||||||
| * TemplateBased instance to resolve translated pages. | ||||||
| * | ||||||
| * @var TemplateBased | ||||||
| */ | ||||||
| protected $templateBasedBlock; | ||||||
|
|
||||||
| /** | ||||||
| * Context View Helper instance to resolve translated pages. | ||||||
| * Constructor | ||||||
| * | ||||||
| * @var Context | ||||||
| * @param TemplateBased $templateBasedBlock TemplateBased instance to resolve translated pages. | ||||||
| * @param Context $contextHelper Context View Helper instance to resolve translated pages. | ||||||
| * @param EscapeHtml $escapeHtmlHelper Escape HTML view helper | ||||||
| * @param Markdown $markdownHelper Markdown view helper | ||||||
| */ | ||||||
| protected $contextHelper; | ||||||
| public function __construct( | ||||||
| #[Autowire(container: \VuFind\ContentBlock\PluginManager::class)] | ||||||
| protected TemplateBased $templateBasedBlock, | ||||||
| #[Autowire(container: 'ViewHelperManager')] | ||||||
| protected Context $contextHelper, | ||||||
| #[Autowire(container: 'ViewHelperManager')] | ||||||
| protected EscapeHtml $escapeHtmlHelper, | ||||||
| #[Autowire(container: 'ViewHelperManager')] | ||||||
| protected Markdown $markdownHelper | ||||||
| ) { | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Constructor | ||||||
| * Make helper invokable. | ||||||
| * | ||||||
| * @param TemplateBased $block TemplateBased ContentBlock | ||||||
| * @param Context $contextHelper Context view helper | ||||||
| * @return static | ||||||
| */ | ||||||
| public function __construct( | ||||||
| TemplateBased $block, | ||||||
| Context $contextHelper | ||||||
| ) { | ||||||
| $this->templateBasedBlock = $block; | ||||||
| $this->contextHelper = $contextHelper; | ||||||
| public function __invoke(): static | ||||||
| { | ||||||
| return $this; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Search for a translated template and render it using a temporary context. | ||||||
| * | ||||||
| * @param string $pageName Name of the page | ||||||
| * @param string $pathPrefix Path where the template should be located | ||||||
| * @param array $context Optional array of context variables | ||||||
| * @param array $pageDetails Optional output variable for additional info | ||||||
| * @param string $pattern Optional file system pattern to search page | ||||||
| * @param string $pageName Name of the page | ||||||
| * @param string $pathPrefix Path where the template should be located | ||||||
| * @param array $context Optional array of context variables | ||||||
| * @param ?array $pageDetails Optional output variable for additional info | ||||||
| * @param ?string $pattern Optional file system pattern to search page | ||||||
| * | ||||||
| * @return string Rendered template output | ||||||
| */ | ||||||
|
|
@@ -90,7 +94,7 @@ public function renderTranslated( | |||||
| array $context = [], | ||||||
| ?array &$pageDetails = [], | ||||||
| ?string $pattern = null | ||||||
| ) { | ||||||
| ): string { | ||||||
| if (!str_ends_with($pathPrefix, '/')) { | ||||||
| $pathPrefix .= '/'; | ||||||
| } | ||||||
|
|
@@ -105,4 +109,25 @@ public function renderTranslated( | |||||
| $context + $pageDetails | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Apply encoding to the content based on the provided content type. | ||||||
| * | ||||||
| * @param string $contentType Content type | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we list legal values here?
Suggested change
|
||||||
| * @param string $content Content | ||||||
| * | ||||||
| * @return string | ||||||
| */ | ||||||
| public function handleContentType(string $contentType, string $content): string | ||||||
| { | ||||||
| if (empty($content)) { | ||||||
| return ''; | ||||||
| } | ||||||
| return match ($contentType) { | ||||||
| 'text' => ($this->escapeHtmlHelper)($content), | ||||||
| 'html' => $content, | ||||||
| 'markdown' => ($this->markdownHelper)(($this->escapeHtmlHelper)($content))->getContent(), | ||||||
| default => throw new InvalidArgumentException('Invalid content type: ' . $contentType), | ||||||
| }; | ||||||
| } | ||||||
| } | ||||||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,6 @@ | |
|
|
||
| namespace VuFind\View\Helper\Root; | ||
|
|
||
| use Laminas\View\Helper\EscapeHtml; | ||
| use Laminas\View\Renderer\PhpRenderer; | ||
| use VuFind\Content\NoticeManager; | ||
| use VuFind\I18n\Translator\TranslatorAwareInterface; | ||
|
|
@@ -63,15 +62,15 @@ class Notices implements TranslatorAwareInterface | |
| * | ||
| * @param NoticeManager $noticeManager Notice manager | ||
| * @param PhpRenderer $renderer PhpRenderer | ||
| * @param EscapeHtml $escapeHtml EscapeHtml view helper | ||
| * @param Content $contentHelper Content view helper | ||
| */ | ||
| public function __construct( | ||
| #[Autowire(service: NoticeManager::class)] | ||
| protected NoticeManager $noticeManager, | ||
| #[Autowire(service: PhpRenderer::class)] | ||
| protected PhpRenderer $renderer, | ||
| #[Autowire(container: 'ViewHelperManager')] | ||
| protected EscapeHtml $escapeHtml | ||
| protected Content $contentHelper, | ||
| ) { | ||
| } | ||
|
|
||
|
|
@@ -123,7 +122,10 @@ public function renderNotice(array $notice): string | |
| if ($content === null) { | ||
| return ''; | ||
| } | ||
| $content = ($this->escapeHtml)($content); | ||
| $content = $this->contentHelper->handleContentType( | ||
| $notice['contentType'] ?? 'text', | ||
| $this->translate($content) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible there might be scenarios where we would not want to translate the content? (Certainly, passing a big chunk of markdown as a translation key isn't going to work -- though it shouldn't cause harm either). Might we want a config setting to explicitly toggle translation, or is that overkill? |
||
| ); | ||
| $classes = ''; | ||
| if ($style = $notice['style'] ?? null) { | ||
| $classes = $this->noticeManager->getConfig()['styles'][$style]['classes'] | ||
|
|
||
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.
We should probably target this branch against dev-12.0 due to the BC-breaking changes to this helper... unless you feel strongly that we should roll this out early with a changelog warning. (I could be persuaded either way; I think it's relatively unlikely that anyone has locally extended the helper, but it's good to be fairly strict about semantic versioning when we can be).