pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
297 stars 444 forks source link

Break out items from article_details.tpl into individual component templates #6880

Open chryslovelace opened 3 years ago

chryslovelace commented 3 years ago

Describe the problem you would like to solve I would like a way to individually refer to items that are included in the article details (such as authors, keywords, citation information, etc.) in templates.

Describe the solution you'd like One solution would be to move each item into its own component template, and to then include those templates in article_details.tpl. This way, those component templates could be individually referenced in other places.

Who is asking for this feature? I am developing a plugin, and this feature would ease and simplify the implementation of that plugin.

Additional information The plugin in question intends to display the contents of an HTML galley directly inline on an article page rather than the article details. I would like to display the information from the article details in blocks on the sidebar, but since the content of the article details template varies based on the theme, it is very difficult to implement these sidebar blocks in a theme-agnostic manner. With this change, I could simply include the component template in each sidebar and allow the theme to define the contents.

NateWr commented 3 years ago

Even if we made this change in the default template, it would have no impact on other themes. You would still face the problem that other themes do not necessarily implement their article landing page in this way.

Have you seen the hooks that are available in the article_details.tpl page? We've tried to use these consistently across all of our themes, so you may have more luck attaching your plugin's content to a hook.

You can see an example from the JatsParserPlugin, which uses a hook to display full text on the article landing page.

This allows you to inject content below the abstract, but it doesn't let you change or remove the other content on the page. There's another hook, Templates::Article::Details, which would let you inject content into the sidebar.

We have long discussed having a block-based configurable layout for the homepage and article landing page. But we haven't made any progress towards this, unfortunately.

ctgraham commented 3 years ago

The point here would be to allow the other theme developers the option of reusing these components rather than hard coding them and allowing the current drift. That is, the presentation of the "How to Cite" in bootstrap will continue to drift from the default theme, unnecessarily and unintentionally and uncontrollably. If we provide core templates for these components, the theme authors get to decide whether they want that drift.

I see this as a two phase enhancement:

NateWr commented 3 years ago

I don't think what you propose will work in the way that you want, @ctgraham. The different HTML code that appears in the bootstrap3 template is not arbitrarily different. That's the HTML code that is necessary to conform to Bootstrap's components.

Replacing:

<!-- Bootstrap -->
<div class="panel panel-default how-to-cite">

With:

<!-- default -->
<div class="item citation">

Will make the block in the Bootstrap3 theme no longer look like a block. The theme also uses special markup for the dropdowns, buttons and other functionality there.

Themes will want to use unique markup for almost every one of the components that could be separated into a different template, because most HTML/CSS/JS frameworks these days are designed around preset HTML that is unique to that framework.

ctgraham commented 3 years ago

At first I was going to concede, but after reflection, I think it will (should) work exactly the way I want.

OJS defines individual templates for article components:

Those templates can be included, excluded, repositioned, etc. by themes as-is. They can also be re-written as-needed by themes (e.g. Bootstrap).

Those templates can also then be re-used by other plugins, to provide the same presentation/function in alternate situations. The intentional divergence of the bootstrap implementation is then a feature rather than a liability.

  1. OJS: a citation tool for an article looks like this ... how_to_cite.tpl
  2. Theme 1: Yes, I want to present that citation tool ... include how_to_cite.tpl
  3. Theme 2: No, I want to override the citation tool presentation ... how_to_cite.tpl
  4. Generic Plugin: I want a citation tool to appear here ... include how_to_cite.tpl

The plugin's inclusion of how_to_cite.tpl delivers the Theme 2's presentation when Theme 2 is enabled, or Theme 1's (default's) presentation when Theme 1 is enabled.

NateWr commented 3 years ago
  1. Theme 2: No, I want to override the citation tool presentation ... how_to_cite.tpl

I don't think this is as common as it might seem. I think that we see child themes that re-order components, but not often ones that replace one component with a different presentation. Usually, if the actual presentation of components is altered much, it is a custom theme entirely.

That's why I think, if we want to go down this route (which would be fairly disruptive and would significantly increase the complexity of theming), we should try to solve the problem in a more robust way by supporting configurable blocks like the website sidebar.

Splitting the templates out like this adds an extra layer of abstraction to the template structure, but only provides a marginal improvement in reusability. In the end, I don't think that we'll reduce our overall maintenance burden.

  1. Generic Plugin: I want a citation tool to appear here ... include how_to_cite.tpl

I'm not aware of any use cases for this. Do you have one? This wouldn't work for how_to_cite.tpl, because the template expects specific JavaScript to be loaded for the interactivity.

ctgraham commented 3 years ago

Does the javascript for "how to cite" really vary by theme? Doesn't this just use the common CSL library?

I think I'm adding confusion by being a bit too abstract.

Problem Statement:

Opportunity:

article_details.tpl

                    <section class="sub_item citation_display">
                        <h2 class="label">
                            {translate key="submission.howToCite"}
                        </h2>
                        <div class="value">
                            {include file="frontend/components/article/howToCite.tpl"}
                        </div>
                    </section>

article_component_howToCite.tpl

                            <div id="citationOutput" role="region" aria-live="polite">
                                {$citation}
                            </div>
                            <div class="citation_formats">
                                <button class="cmp_button citation_formats_button" aria-controls="cslCitationFormats" aria-expanded="false" data-csl-dropdown="true">
                                    {translate key="submission.howToCite.citationFormats"}
                                </button>
                                <div id="cslCitationFormats" class="citation_formats_list" aria-hidden="true">
                                    <ul class="citation_formats_styles">
                                        {foreach from=$citationStyles item="citationStyle"}
                                            <li>
                                                <a
                                                    aria-controls="citationOutput"
                                                    href="{url page="citationstylelanguage" op="get" path=$citationStyle.id params=$citationArgs}"
                                                    data-load-citation
                                                    data-json-href="{url page="citationstylelanguage" op="get" path=$citationStyle.id params=$citationArgsJson}"
                                                >
                                                    {$citationStyle.title|escape}
                                                </a>
                                            </li>
                                        {/foreach}
                                    </ul>
                                    {if count($citationDownloads)}
                                        <div class="label">
                                            {translate key="submission.howToCite.downloadCitation"}
                                        </div>
                                        <ul class="citation_formats_styles">
                                            {foreach from=$citationDownloads item="citationDownload"}
                                                <li>
                                                    <a href="{url page="citationstylelanguage" op="download" path=$citationDownload.id params=$citationArgs}">
                                                        <span class="fa fa-download"></span>
                                                        {$citationDownload.title|escape}
                                                    </a>
                                                </li>
                                            {/foreach}
                                        </ul>
                                    {/if}
                                </div>
                            </div>

Use case 1: Default

Use case 2: Existing theme

Use case 3: Existing theme with updates (to include upstream)

Use case 4: Existing theme with updates (to override upstream)

Use case 5: New generic plugin which wants to expose an article component in a novel way, with compatibility across themes.

This also positions us well for a future of converting these into blocks within core, as the intellectual work of identifying them as components is already done.