-
Notifications
You must be signed in to change notification settings - Fork 7
Conversation
'content' => [$contentHeader], | ||
] | ||
) | ||
$this->contentHandler->handle($dom->documentElement, $context) |
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.
Other option is not have separate controllers for each type.
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.
This has been updated to use a single controller, it seems
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.
Ah, s/not/to
...
vendor-extra/ContentPageBundle/src/Handler/JatsContentHandler.php
Outdated
Show resolved
Hide resolved
const DIRECTION_LTR = 'ltr'; | ||
const DIRECTION_RTL = 'rtl'; | ||
|
||
function text_direction(string $locale) : string |
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.
Should be extracted from this PR. Not entirely sure of the best place for it...
|
||
</front> | ||
</meta> | ||
|
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.
the wrapper XML format itself makes sense
use function array_merge; | ||
use function Libero\ContentPageBundle\text_direction; | ||
|
||
final class JatsContentHandler implements ContentHandler |
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.
the name JatsContentHandler
(and perhaps JatsContentBundle
) is misleading if, as far as JATS variations go, this can support eLife-like JATS or Libero-constrained JATS rather than the totality of JATS content. This makes a conversion step still necessary from non-Libero-constrained JATS to Libero-constrained JATS, unless this frontend and all the various services make efforts to support various options.
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.
On the ingestion validation side of things, I guess a Schema(-tron?) could be written to validate it's Libero-constrained JATS, don't know how difficult it is to do
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.
Indeed, not yet clear what JATS will be supported yet. I suspect it's going to be somewhere between DAR and JATS4R. There might be a certain amount of configuration too, so you could tweak selectors.
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.
Where is the concept different from the original eLife 1.0 JATS-to-HTML XSLT, but done with code? For sure, it uses patterns rather than producing HTML directly
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.
It's not. You probably could use XSLT to do the same (presumably producing JSON which is a valid input for the patterns), but I suspect there's things not possible there (eg having to replace placeholders in code afterwards).
vendor-extra/ContentPageBundle/src/Handler/LiberoContentHandler.php
Outdated
Show resolved
Hide resolved
use Symfony\Component\DependencyInjection\Reference; | ||
use function array_keys; | ||
|
||
final class ContentHandlerPass implements CompilerPassInterface |
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.
these Passes
could do with a docblock describing how they map to the YAML schema, but maybe this is already described elsewhere
@@ -47,6 +47,7 @@ private function addPage(string $name, array $config, ContainerBuilder $containe | |||
$definition->setArgument(2, new Reference('twig')); | |||
$definition->setArgument(3, $config['page_template']); | |||
$definition->addTag('controller.service_arguments'); | |||
$definition->addTag('libero.content_page.controller', ['handler' => $config['handler']]); |
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.
handler
is overloaded, we'll find a metaphor for the naming I suppose, given publishing wealth of concepts and strange words
Complementary to #29, and based on the possible idea of not requiring content items to be a
{http://libero.pub}item
.This changes the scholarly article example to be
{http://jats.nlm.nih.gov}article
.