Skip to content
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

[OJS] include journal info on index journal page #1864

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

carzamora
Copy link
Contributor

the journal page only allows to show additional content at the end of the page, after showing the contents, I think it is good to show some information before displaying the contents, such as a brief introduction to the journal. (I know this change can be added to a custom template, but for the importance, I propose it for the default template, I think is good for SEO too like brief site description on index page)

the journal page only allows to show additional content at the end of the page, after showing the contents, I think it is good to show some information before displaying the contents, such as a brief introduction to the journal. (I know this change can be added to a custom template, but for the importance, I propose it for the default template, I think is good for SEO too like brief site description on index page)
@asmecher asmecher requested a review from NateWr March 8, 2018 22:42
@asmecher
Copy link
Member

asmecher commented Mar 8, 2018

Mind taking a look, @NateWr?

@NateWr
Copy link
Contributor

NateWr commented Mar 12, 2018

It makes sense that some journals will want this displayed on the homepage. But I'm hesitant to introduce this change to all of our journals without strong demand for it. It will tamper with journals' existing homepage in ways that some journals won't like.

Ideally, I'd like to see this use the theme options API to be configurable per journal (off-by-default). But we also don't want to introduce things into the core templates which rely on a theme option which may not be present in other themes.

One possibility is to introduce it as a theme option, but then to have the theme inject it via the hook in the indexJournal page: https://github.com/pkp/ojs/blob/master/templates/frontend/pages/indexJournal.tpl#L23. It feels a bit like a round-about way to do it and it could make things more difficult for themers wanting to hack at the default theme. But it will keep the core template clean for now, until we have a better technique for making the homepage more configurable out-of-the-box while keeping things simple for third-party themes.

@carzamora
Copy link
Contributor Author

carzamora commented Mar 14, 2018

I understand your vision, I had this idea when I saw the "Bootstrap 3 Base Theme", this themes have this block of info at above the content issue. I do not understand very well the pros or against this, so ... keep it in mind for later if possible.
In my case, to modify this, I modified the child theme "Manuscript" for show more info about the journal. 😉

Base automatically changed from master to main February 18, 2021 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants