-
Notifications
You must be signed in to change notification settings - Fork 546
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
Allow customization of frontpage text #716
Conversation
Any news on this PR? I'd be happy to help if any adjustments might be required to get it into master. |
Sorry. I have not taken the time to look at any of the nbviewer PRs. I'll try to get to them in the near future if someone else doesn't get here first. |
Any news? I'd love to help get this into the repo. If there are any blockers, please let me know if I can do anything to help remove them. |
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.
Just one question otherwise it seems reasonable to me.
@@ -1,10 +1,15 @@ | |||
{% extends "layout.html" %} | |||
|
|||
{% block body %} | |||
{% if not no_frontpage_input %} |
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 origin no_frontpage_input
flag also hid the jumbotron header. Does the page look OK with that element still in the DOM when there's no title, subtitle, or text?
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.
Let me check on that, and get back to you.
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.
I've added a new commit to hide the jumbotron if nothing is contained within it.
As a result, the jumbotron is completely hidden if none of title, subtitle, text are set and show_input is False
Looking back, I intended that the title field would be required, so hiding the jumbotron wasn't an issue. However, given that you could hide it before, I've added another commit here to make it no longer required, and to hide the jumbotron itself if nothing is being shown in it. Thanks for your help getting this done! |
Looks good now. Thanks for the feature @dbdean. |
Following on from my earlier pull request (#683), I've added top-level 'title', 'subtitle', 'text' and 'show_input' fields to the frontpage.json file, with the original sections under a top-level 'sections' tag.
Current-style frontpage.json files with only the list of sections will still work as they did before, but may be considered deprecated with this PR. I have updated the test cases to have a new schema that validates the new frontpage.json style, and the tests pass.
This has allowed me to fully customize the text in the front page for my organisation without having to maintain a separate fork, although I may need to do so when I want to change the styling (one thing at a time!).
I have remove the --no-frontpage-input option that I introduced in #683, as it is no longer needed, and I am assuming it was just me using it. If needed, it could be kept with a deprecation warning.
I'm happy to modify this pull request as needed to get it into the main repository.
Thanks!