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

Add more blocks to Simple theme's base template #3405

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

frederik-elwert
Copy link
Contributor

@frederik-elwert frederik-elwert commented Oct 10, 2024

This change makes it easier to create new themes by inheriting from the simple theme. It allows customization of the whole body (while still making use of the theme’s head), or individual parts of the body like header, menu, or footer.

Pull Request Checklist

  • Ensured tests pass and (if applicable) updated functional test output
  • Conformed to code style guidelines by running appropriate linting tools
  • Added tests for changed code
  • Updated documentation for changed code

@justinmayer justinmayer requested a review from pauloxnet October 17, 2024 16:15
@justinmayer
Copy link
Member

Any feedback on this PR from @getpelican/reviewers?

@boxydog
Copy link
Contributor

boxydog commented Nov 3, 2024

It is traditional to have blocks for header and footer, so that seems like a decent idea. It might also be traditional to separate them out into footer.html and header.html, though I wouldn't insist.

I don't see customization of the menu, although that is mentioned in the description of this PR.

My bigger questions:

  1. Is this actually useful for someone? What is the use case?
  2. If yes, then is it possible to write a test?

@frederik-elwert
Copy link
Contributor Author

Sorry for not getting back to this earlier. Thanks for the comments.

It is traditional to have blocks for header and footer, so that seems like a decent idea. It might also be traditional to separate them out into footer.html and header.html, though I wouldn't insist.

Yes, separating them out is also a good idea, and it would make it easy to override them without even touching the base template. It would require a bit more understanding of the file layout, though, so for beginners having a single file to override might even be simpler. But I’m open to both suggestions.

A slightly related question: At the moment, the blocks are defined within the corresponding elements, i.e., there’s a <footer>, and inside that a {% block footer %}. I followed the model for head here. But the consequence is that even when overriding the footer block, one is still bound to having that footer tag around it. (It would still be possible to override the complete body block, though.) Would it make sense to switch that and have the elements within the block? (The same would apply to includes if one were to factor them out into separate files.)

I don't see customization of the menu, although that is mentioned in the description of this PR.

That’s the block nav that I was referring to.

My bigger questions:

1. Is this actually useful for someone? What is the use case?

I’m teaching SSGs with Pelican to university students, and one issue is customizing the appearance without having to write a full theme. My understanding for the existence of the simple theme was that it would be a starting point for simple customizations, which is what the docs suggest. The reason for the PR is that customization works well for the use case in the docs (mainly providing a custom CSS file), but going beyond that requires a customized copy of the base template. This PR would offer a middle way of reusing the simple theme, but allowing more customization.

And I think it would even be useful for theme authors. (I’m currently writing a simple custom theme that basically uses this mechanism.)

2. If yes, then is it possible to write a test?

I’m happy to provide a test, but I’m not really sure what the test would cover?

@boxydog
Copy link
Contributor

boxydog commented Nov 28, 2024

I’m teaching SSGs with Pelican to university students

Sounds good. What's an SSG?

I’m happy to provide a test, but I’m not really sure what the test would cover?

Literally any line of code that you changed. For example, override the footer and test that the new footer shows up.

@justinmayer
Copy link
Member

SSG is short for static site generator.

@pauloxnet
Copy link
Member

Any feedback on this PR from @getpelican/reviewers?

Please add explicit name in endblocks

Copy link
Member

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for this enhancement, Frederik! Looks good to me.

Any last comments from @getpelican/reviewers? If possible, I would like to merge this today and ship a new release with the other accumulated improvements.

Frederik Elwert added 3 commits January 15, 2025 11:58
This change makes it easier to create new themes by inheriting from the
simple theme. It allows customization of the whole body (while still
making use of the theme’s head), or individual parts of the body like
header, menu, or footer.
@frederik-elwert
Copy link
Contributor Author

Thanks for the feedback.

  • I added explicit endblock names.
  • I don’t think there already were tests in place for the simple theme and inheritance. I’m not very familiar with code testing, and in this case it also requires some i/o, but with the help of Claude AI (but manually checked and updated by me), I created a basic test for the simple theme and theme inheritance based on the "simple_content" test data. I hope this goes into the right direction.
  • I just rebase the branch onto main and squashed a few extra commits.

Copy link
Member

@pauloxnet pauloxnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding endblock names.

@justinmayer justinmayer changed the title Add more blocks to simple base template Add more blocks to Simple theme's base template Jan 15, 2025
@justinmayer
Copy link
Member

Thanks again to @frederik-elwert for the contribution and to @boxydog and @pauloxnet for reviewing ✨

@justinmayer justinmayer merged commit 64be147 into getpelican:main Jan 15, 2025
18 checks passed
@frederik-elwert frederik-elwert deleted the improve-simple-theme branch January 15, 2025 11:43
@boxydog
Copy link
Contributor

boxydog commented Jan 15, 2025

I created a basic test for the simple theme and theme inheritance

Fantastic, thank you.

@justinmayer
Copy link
Member

I just released Pelican 4.11.0, which includes the changes in this pull request.

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.

4 participants