Skip to content

Re-order the theme template structure to be closer to sphinx-basic-ng #577

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

Closed
3 of 5 tasks
Tracked by #610
choldgraf opened this issue Jan 26, 2022 · 7 comments
Closed
3 of 5 tasks
Tracked by #610
Labels
kind: maintenance Improving maintainability and reducing technical debt

Comments

@choldgraf
Copy link
Collaborator

choldgraf commented Jan 26, 2022

Description

(for this issue I'm defining sections as major areas of the theme UI, and components as subsets of HTML within those sections)

Currently, our HTML template structure is a bit haphazard, we have:

It's a bit hard to figure out what template should be in the root, and what should be in the _templates folder. We also don't follow any kind of standard naming convention for our sections. This makes our theme more idiosynchratic to contribute to and maintain.

Proposal

We could adopt the same template naming and structure conventions that the sphinx-basic-ng theme uses. This would be something like:

layout.html  <-- major structure of the page
sections/  <-- folder for templates of major sections of the page
components/ <-- folder for smaller page components that may be re-used as well

Moreover it would mean naming our major UI sections the same as the sphinx-basic-ng, so for example our "navbar" would be renamed to sections/header.html and our "banner" would be renamed to sections/announcement.html.

This would standardize our theme structure a bit more, and is another baby step towards directly using the sphinx-basic-ng theme as an ancestor if we wish.

Implementation guide

Note that this would also break a lot of pre-existing PRs, so we might want to merge those first before doing work here if we want to do this.

Tasks

  • Identify any PRs that we want to merge before this happens (to avoid annoying merge conflicts)
  • Implement the layout reorganization: Refactor SCSS structure #619
  • Refactor our HTML templates to use sections/, components/, etc folders, and separate out the layout.html into those sections
  • Potentially add CSS classes / HTML elements to our layout.html to match the semantics of the basic-ng theme (e.g., sidebar-primary, sidebar-secondary etc)
@choldgraf choldgraf added the kind: maintenance Improving maintainability and reducing technical debt label Jan 26, 2022
@damianavila
Copy link
Collaborator

I am fully supportive of this reorganization. Even if we never migrate to sphinx-basic-ng, it would be super useful to have the templates better organized and "compatible" with other implementations.

@jorisvandenbossche
Copy link
Member

Generally +1 on better standardizing / structuring our templates!
(I think at some point we started adding new ones in _templates, leading to this mixture of some in that folder and some the level above ..)

our "banner" would be renamed to sections/announcement.html.

Do you mean the html div with banner id:

{# Added to support a banner with an alert #}
<div class="container-fluid" id="banner"></div>

(because I don't see that we have a banner template file?)

Changing that would break users though.

In general, changing the names of templates is also in theory a breaking change? (I think a downstream user can eg have a layout.html where they also refer to our templates?)

@choldgraf
Copy link
Collaborator Author

Do you mean the html div with banner id:

Yep - though I wasn't able to find a place where we make it possible to actually use that banner. So I think this would be more like "activating" that feature in the first place, rather than breaking existing functionality.

In general, changing the names of templates is also in theory a breaking change

In theory yes. We wouldn't change the layout.html filename, though potentially would change its structure. And for the other template files they'd change their location, so that would be a breaking change as well.

That said, it would be a breaking change for people who are pretty tech savvy (who are able to over-ride Sphinx templates on their own) so I'd hope the impact could be minimized.

@damianavila
Copy link
Collaborator

I have merged #619 a few minutes ago, so let's close this one now!

Repository owner moved this from Todo to Done in Developer workflow improvements Apr 20, 2022
@choldgraf
Copy link
Collaborator Author

choldgraf commented Apr 21, 2022

@damianavila I think this one is not quite yet done - we've refactored the SCSS, but the theme's HTML structure is still the same. I think the _templates folder structured described above has not yet been implemented.

IMO there are still two things that need to happen:

  1. Refactor our HTML templates to use sections/, components/, etc folders, and separate out the layout.html into those sections.
  2. Potentially add CSS classes / HTML elements to our layout.html to match the semantics of the basic-ng theme (e.g., sidebar-primary, sidebar-secondary etc)

@choldgraf choldgraf reopened this Apr 21, 2022
@damianavila
Copy link
Collaborator

IMO there are still two things that need to happen:

Update the top comment with these steps!
I closed it prematurely when I saw the previous two items already checked and the corresponding PRs merged, my bad (it was a long day 😉).

@damianavila
Copy link
Collaborator

damianavila commented May 25, 2022

This one was addressed by #662, so closing this one now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: maintenance Improving maintainability and reducing technical debt
Projects
Development

No branches or pull requests

3 participants