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

Templates: site footer #46

Open
alisonjo315 opened this issue Oct 20, 2021 · 1 comment · May be fixed by #57
Open

Templates: site footer #46

alisonjo315 opened this issue Oct 20, 2021 · 1 comment · May be fixed by #57
Assignees

Comments

@alisonjo315
Copy link
Collaborator

alisonjo315 commented Oct 20, 2021

Our norm these days is to put site footer "stuff" in html.html.twig (usually by way of a partial/include template called site-footer.html.twig).

But, cwd_base puts it in page.html.twig.

I recommend we move this code to html.html.twig!

Reasons:

  • Because that's our norm these days.
  • It's messy for us to do it that way in child themes, and still have it in page.html.twig in cwd_base -- for example, we wanted to put footer code into a site-footer partial template in cwd_project (included in html.html.twig), but we can't, unless we add page.html.twig to cwd_project -- and we have no other need for page.html.twig in cwd_project, so it's kind of a bummer.

Problem: Not backwards-compatible 😕
If an existing site uses cwd_base/page.html.twig (i.e. if they don't have a template override in their child theme*), their footer will get messed up if they update cwd_base.

  • But, we're pretty clear with everybody that cwd_base updates are not necessarily backwards compatible.
  • And this change could be easily remediated on a child site that wants to update cwd_base without breakage (i.e. create a copy of "old" page.html.twig in their child theme, that's it).
    • We can even say this in the release notes.
  • And, let's be real, is any site NOT overriding cwd_base/page.html.twig??

* We don't need to worry about child themes using cwd_base/page.html.twig with block overrides or include variables, because it doesn't have any such things.

@alisonjo315
Copy link
Collaborator Author

I think we should go for it.

we're pretty clear with everybody that cwd_base updates are not necessarily backwards compatible.

And, we can put a big note in the release notes. If we want, we can even bump the semantic version up to 3.x
^^ If we do that, we could take some other liberties, like...

  • rename "includes" => "partials" (to match cwd_project)
  • do the non-paragraphs "slideshow" template work right here in cwd_base (instead of cwd_project) (related issue on CD Demo)
  • ...other breaking changes?

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 a pull request may close this issue.

1 participant