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

support various footer configurations #225

Merged

Conversation

jdknight
Copy link
Contributor

Adjust the injection of separators between various components of a footer to only do so when needed. This is to handle various configuration states a user may set for html_last_updated_fmt, html_show_copyright, html_show_sourcelink or html_show_sphinx.


A capture of various configuration modes:

image


Note that with the provides changes, there are two minor tweaks to be considered before accepting this change.

1)
The period is purposely not injected at the end of a copyright statement. This provides flexibility for documentations which may not desire to inject a period after its statement (assuming there is no trailing content to append in the footer). For example:

image

2)
The vertical spacer was omitted in these changes to opt for using the existing period entries as the separator (not including the "Show Source" entry. Below shows the existing pip documentation -- and when comping to the above figure, the difference can be observed:

image

If this is not desired, the merge request can be modified.


See also: #220

@pradyunsg
Copy link
Owner

pradyunsg commented Aug 29, 2021

Thanks for filing this PR!

I like this overall. Periods are certainly less visually noisy than the pipe.

This provides flexibility for documentations which may not desire to inject a period after its statement (assuming there is no trailing content to append in the footer)

I don't think this flexibility is necessary. It's not provided today and I'm not convinced that there is a strong reason to provide it.

I'd prefer to switch to using period as the separator everywhere OR sticking to pipes everywhere.

@jdknight
Copy link
Contributor Author

jdknight commented Aug 30, 2021

I don't think [optional period] flexibility is necessary.

Sure. I can adjust it to restore a fixed period character.

I'd prefer to switch to using period as the separator everywhere OR sticking to pipes everywhere.

I am good either way and am willing to make the adjustments in this pull request, if desired. I am just not sure which approach would want to be taken, as it is styling preference that I assume you would rather make the call on. From what I can tell, it could either be:

approach 1

The period-like style submitted in this pull request can be used (aside from corrections to be made, already noted above). If all pipes were to be removed, is there any other additional changes to consider for the "Show Source" section?

image

It is assumed that just removing the pipe would be fine enough; however, I did not want to make any assumptions here.

approach 2

The pipes can be restored back to its original design:

image

And the pull request can be adjusted to solely focus on the removal of the pipe characters if specific sections are disabled via the configuration.

@dgw
Copy link
Contributor

dgw commented Aug 30, 2021

(I am just a user of the theme but) I slightly prefer the original design with pipes. The pipeless design is great too, as long as it's consistent. (So I also agree with @pradyunsg—consistency is what's really important.)

Adjust the injection of separators between various components of a
footer to only do so when needed. This is to handle various
configuration states a user may set for `html_last_updated_fmt`,
`html_show_copyright`, `html_show_sourcelink` or `html_show_sphinx`.

Signed-off-by: James Knight <james.d.knight@live.com>
@jdknight jdknight force-pushed the support-various-footer-configurations branch from 87f4688 to e62550b Compare September 5, 2021 23:58
@jdknight
Copy link
Contributor Author

jdknight commented Sep 6, 2021

Adjusted the changes back to using the original pipe styling, but only applying pipes to inject if a leading section has been rendered. Example shots:

image

@pradyunsg pradyunsg merged commit fcb2e94 into pradyunsg:main Sep 7, 2021
@pradyunsg
Copy link
Owner

Thanks @jdknight! ^.^

@jdknight jdknight deleted the support-various-footer-configurations branch September 7, 2021 12:41
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.

None yet

3 participants