Custom content slots for landing page, about page and footer#1817
Custom content slots for landing page, about page and footer#1817
Conversation
…ontent boxes on landing page
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1817 +/- ##
============================================
+ Coverage 70.30% 70.39% +0.08%
- Complexity 1649 1651 +2
============================================
Files 34 34
Lines 4328 4340 +12
============================================
+ Hits 3043 3055 +12
Misses 1285 1285 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…r slots/landing-end/
d8d41bb to
bfd9de5
Compare
|
TODO:
|
…stom-templates/footer/
…t page content to custom-templates/about/
…ing overriding of built-in Twig templates
…own custom content from version control
…ise for translation)
There was a problem hiding this comment.
Some general comments:
The note Adding an empty Twig file will remove this message is clear and straightforward instruction for removing a message that the developer might consider unnecessary. It could hardly be any easier to remove — well done!
With this approach we are getting closer to a publishing system - which is a good thing within reasonable limits. I thing we are still within those limits, since the easy customizability of Skosmos’s structure and appearance is surely welcome from the developers’ perspective. And after all, developers still have the possibility to modify Skosmos as they want.
miguelvaara
left a comment
There was a problem hiding this comment.
Looks like it works perfectly in every possible use case. I tested all possible use cases and also tried using our own CSS style.
Just some example:
A really good improvement! It will likely make our own developing work a bit easier in the future and definitely help external developers as well ,since adding and removing slots is truly simple and effortless. Are you writing the documentation before or after the merge?
miguelvaara
left a comment
There was a problem hiding this comment.
All tests and syntax checks passed. Once the timing question of the documentation is solved and you have chosen whether the the directory existence should be checked in the WebController's code, the branch can be merged. Excellent work!
…irectory doesn't exist
|
|
Wrote up some wiki documentation: https://github.com/NatLibFi/Skosmos/wiki/Custom-templates I also resolved the merge conflict for |
miguelvaara
left a comment
There was a problem hiding this comment.
We will probably need to discuss the length of translation message keys at some point later on. The recommendation is that in the default file the key and the message are the same, but that can sometimes make the code rather “interesting” ;-) to read. So the question is -- do we want to do it by the book or make it look good?
Push to Lokalise after the merge?
Everything looks great!
Post scriptum:
A small curiosity, nothing that needs action:
“Du kan ta bort detta meddelande genom att lägga till en tom twig-fil.”
Might “det här” (instead of “detta”) sound a bit too Swedish from a Finland-Swedish perspective? Maybe - maybe not? No need to get stuck on this, I just found myself wondering about it.
|
Translations have already been pushed to Lokalise (including the long message). We can talk about that but I think it's out of scope for this PR. Likewise the Swedish translation: we can change that in Lokalise later if we need to; I think many such adjustments will still be needed down the line. Merging this as it is now, thank you for the review! |
Reasons for creating this PR
Implement customizable content slots on the landing and about pages as well as the footer that is displayed on all pages. See #1815.
This PR also removes our own custom content such as the "Welcome to the Skosmos browser demo" and "Skosmos is open source and on GitHub" boxes, the footer, and the about page content. We don't want these to be included with the Skosmos codebase since they are not useful for other instances. They should be stored separately, just like the configuration files.
Link to relevant issue(s), if any
Description of the changes in this PR
custom-templateswith subdirectorieslanding-top,landing-start,landing-end,landing-bottom,about,footer,html-headlanding-top-slot,landing-start-slot,landing-end-slotandlanding-bottom-slotto thelanding.twigtemplate and similar adjustments toabout.twigandbase-template.twigcustomTemplates, populate the above mentioned DIVs by including any Twig template files fromcustom-templates/*/*.twig, in alphabetical order; in some cases (landing-end-slot and about-slot), show a placeholder message with instructions if no templates have been defined.Here is an example of the placeholder message shown on the landing page:
Known problems or uncertainties in this PR
Checklist
.sr-onlyclass, color contrast)