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

Reworking theme structure to use components #8

Closed
wants to merge 1 commit into from

Conversation

choldgraf
Copy link
Contributor

@choldgraf choldgraf commented Mar 15, 2021

This re-works a bit of the theme structure to use sections and components. It makes the components that live in each section configurable, and if sections have no components then they are not included in the end-result. It also uses @mgeier's approach of letting you define defaults for component lists as a string, and then splitting them 🎉

It does the following:

  • Adds theme configuration for each component (in html_theme_options, these behave similarly to the sphinx sidebar config, though there's no pagename: templates functionality yet)
  • Adds a bunch of "demo components" to highlight the different theme elements
  • Does some class renaming to make theme sections be more about their place in the theme, than the semantics of their use
  • The simple test site you shared now uses this config so it should work out of the box with nox

To Do

(Either here or in some other pr)

  • Decide whether these are reasonable design decisions for the theme
  • Maybe remove some of the changes that others don't agree w/!
  • Make any tweaks to names, design, etc
  • Make sure the CSS still works as expected (I think the content-sidebar, now called sidebar-right needs some fixing)
  • Make sure the demo docs still work w/ this structure
  • do we need a content footer? I feel like that can just be "the last component in the middle-content section"
  • I think the right sidebar and the middle content area should have the same minimum height, w right sidebar snapped to the right. Then the middle content area scrolls while the right sidebar remains static

@pradyunsg
Copy link
Owner

As I noted in #7, I really really really don't want to be using theme_options for this. I don't think this theme should expose knobs that are capable of so drastically changing how the theme looks, to the end user.

topbar_middle =
topbar_right =
middle_content = "components/page-content.html"
sidebar_left = "components/site-logo.html, components/site-nav.html"
Copy link

Choose a reason for hiding this comment

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

There shouldn't be a need for quotes around this.
In fact, there probably shouldn't be any quotes at all in conf.py.

I'm not sure about the path separators, though, I've never used sub-directories ...

@mgeier
Copy link

mgeier commented Mar 15, 2021

It also uses @mgeier's approach of letting you define defaults for component lists as a string, and then splitting them 🎉

AFAICT, no quotes are necessary in theme.conf, the values right of the equals signs are implicitly strings.

Also, my main idea was to make this work without needing any Python extension code (only pseudo-Python within Jinja tags).

I think a Sphinx theme shouldn't have to be a Sphinx extension as well. But I guess you need that for other stuff anyway, right?

@choldgraf
Copy link
Contributor Author

choldgraf commented Mar 15, 2021

@pradyunsg just to clarify - don't treat the specific implementation details in this PR as strong opinions, I just tried to put together the minimal PR that showed the kind of functionality I had in mind.

That said I imagined that sub themes wouldn't document these configuration values unless they wanted to. So primarily a sub theme would just override the values for one of the components lists, and that'd be it. If not documented, I highly doubt a user would ever know it existed. Only if the author wished they would document it and thus support it.

That said I imagine this could live in a sphinx config like components or theme_structure or something, but putting it in html_theme_options was a faster path to an MVP.

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.

3 participants