Upgrade readthedocs theme to v1.0.0 #2585

Sparticuz · 2021-09-23T18:55:34Z

I've gone through each release we've missed on the readthedocs theme, and updated the mkdocs fork. I've mainly kept to just implementing the changes as able and not rewrote functionality, (see Copyright in mkdocs fork vs upstream).

There are a few features that I've not implemented, due to them missing in our original fork, or just for ease of upgrading our fork:

The ability to use a logo in place of site_name in base.html mkdocs fork, upstream Planned Done in 3cf33d9
Adding the version under the site_name in base.html, upstream, Not exactly sure what this version is pointing to, is it a git tag, or a config option, etc...? Might be related to Support multiple versions of docs. #193 Added support in 4c66a1a, but it just displays a string
Article comments in base.html under page.content, upstream.
Build info in footer.html, upstream. I would like to implement this, but I believe I would need to add the mkdocs-git-revision-date-localized-plugin plugin
~~Upstream has deprecated and removed the 'isogram' version of google analytics, but I wanted comment here before I also removed it.~~ Keeping isogram for now

Fixes #2538
Fixes #2537
Fixes #2467
Fixes #1763
Fixes #2239

readthedocs/sphinx_rtd_theme@0.4.1...0.4.2

readthedocs/sphinx_rtd_theme@0.4.2...0.4.3

Sparticuz · 2021-09-23T19:02:06Z

Refs at least these #2538, #2537, #2467, #1763, maybe more?

EDIT: #2239

This was moved to a package.json in upstream for version 0.5.0rc1, then upgraded to 3.6.0 in 0.5.2

Sparticuz · 2021-09-23T19:38:35Z

Example of logo image, as well as style_nav_header_background

Sparticuz · 2021-09-23T20:09:25Z

I've added support in 4c66a1a for

extra:
  version: "string"

It's just a string, doesn't pull anything from git.

oprypin

This is all great work, thank you very much!

I have checked all the content of this pull request and it all makes sense and corresponds well to the actual changes in sphinx_rtd_theme.

I have only left small comments.

Now I'm just thinking how we can involve more people to try out this theme to make sure it actually works well on the frontend side.

mkdocs/themes/readthedocs/mkdocs_theme.yml

mkdocs/themes/readthedocs/base.html

mkdocs/themes/readthedocs/mkdocs_theme.yml

This is being done in favor of using a css setting for `.wy-side-nav-search` Update config_tests.py

oprypin · 2021-09-27T18:11:09Z

I was strongly considering merging this PR with individual commits, but now I think it's slightly less organized for that. And, the main reason is that the commits contain large files which we may as well omit from storing in the repo. So I think I'll squash.

Also did you see that CI is failing?

Sparticuz · 2021-09-27T18:45:14Z

I had kindof planned on it being squashed. Most of the commits are really unneeded, but it helped me visualize the changes by going release by release.

I did see the CI was failing, but I have no idea why. The error it's giving doesn't make any sense to me because the IP it's giving IS an IPv4 address.... It's also unrelated to any changes I made.

======================================================================
ERROR: test_IP_normalization (tests.config.config_options_tests.IpAddressTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/mkdocs/mkdocs/mkdocs/config/config_options.py", line 258, in run_validation
    host = str(ipaddress.ip_address(host))
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/ipaddress.py", line 53, in ip_address
    raise ValueError('%r does not appear to be an IPv4 or IPv6 address' %
ValueError: '127.000.000.001' does not appear to be an IPv4 or IPv6 address

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/work/mkdocs/mkdocs/mkdocs/tests/config/config_options_tests.py", line 207, in test_IP_normalization
    value = option.validate(None)
  File "/home/runner/work/mkdocs/mkdocs/mkdocs/config/config_options.py", line 130, in validate
    return self.run_validation(value)
  File "/home/runner/work/mkdocs/mkdocs/mkdocs/config/config_options.py", line 260, in run_validation
    raise ValidationError(e)
mkdocs.config.base.ValidationError: '127.000.000.001' does not appear to be an IPv4 or IPv6 address

pawamoy · 2021-09-27T19:10:27Z

It's possible that GitHub Actions upgraded their 3.8 version to a later patch that changed how IP addresses are normalized. See the "changed in" notes in https://docs.python.org/3/library/ipaddress.html#ipaddress.IPv4Address

oprypin · 2021-09-27T22:53:56Z

OK! Yes, that fails on 'master', reproduced it here: https://github.com/mkdocs/mkdocs/runs/3723870272 and will fix it here: #2587

oprypin · 2021-09-27T22:55:27Z

I have not decided when to merge it, and whether it's appropriate for a patch release, but this is definitely accepted.

mkdocs/themes/readthedocs/mkdocs_theme.yml

Sparticuz · 2021-09-28T13:17:14Z

I agree, I don't think this is a patch release. There were very few structural changes to the HTML, just a few things added. Mainly for the logo and version in the header, which are both optional additions. Actually the largest change was probably dropping Modernizr and replacing it with html5shiv. I guess, technically, that would have the possibility of breaking changes if someone were using the Modernizr CSS classes in their css-extra.css file.

For #2467, my PR only addresses the readthedocs theme. Would you like me to also add a fix for the mkdocs theme, or would you prefer a separate PR? (I could also take a look at updating that theme as well)

What are your thoughts on 3 and 4 above?

For 3, It doesn't appear that MkDocs supports comments, but that doesn't necessarily mean that someone couldn't use something like disqus with a override on the comments block? Adding this should be trivial.

For 4, That would add this stuff

to this area of our fork

IMO, this might be better as a separate PR, as it might need to add in a few plugins

oprypin · 2021-09-28T16:26:51Z

I agree, I don't think this is a patch release. There were very few structural changes to the HTML, just a few things added.

Uhh I think there's some misunderstanding here. I was thinking whether it's appropriate for 1.2.3 (patch release) or 1.3.0. So which one do you mean?

For #2467, my PR only addresses the readthedocs theme. Would you like me to also add a fix for the mkdocs theme, or would you prefer a separate PR?

You're correct. Definitely a separate PR would be best. Thank you if you take up that work.

(3) and (4) I have not evaluated, but definitely let's not do those in this PR

Sparticuz · 2021-09-28T17:58:06Z

I agree, I don't think this is a patch release. There were very few structural changes to the HTML, just a few things added.

Uhh I think there's some misunderstanding here. I was thinking whether it's appropriate for 1.2.3 (patch release) or 1.3.0. So which one do you mean?

I think it's a minor release, not a patch release.

oprypin · 2021-09-28T20:23:35Z

@Sparticuz I would agree that the naming is unfortunate, but my use of it was correct as per https://semver.org/

But regardless, could you clarify your opinion, do you think it's appropriate for 1.2.3?

Sparticuz · 2021-09-29T13:45:29Z

@Sparticuz I would agree that the naming is unfortunate, but my use of it was correct as per https://semver.org/

But regardless, could you clarify your opinion, do you think it's appropriate for 1.2.3?

Talking for the Theme itself, according to semver, it's a major release. Modernizr got removed, which removes a bunch of css classes that get added by the modernizr javascript. If a user was using those css classes, it would be a breaking change. (Also jquery went from v2 to v3, but I'm not sure jquery adheres to semver)

As for the version of MkDocs, it's a decision of whether or not a major update of a dependency justifies a major update of MkDocs itself. I'm not familiar with what Semver says for a breaking change in a dependency. (If we consider the readthedocs theme a dependency of MkDocs)

If you wanted, I could add in

theme:
  name: readthedocs
  modernizr: True

which would keep modernizr. I'm not familiar with the local dev server, but maybe a deprecation warning could be added if config.theme.modernizr was True. Then when MkDocs v2 is published, remove modernizr entirely.

oprypin · 2021-09-29T15:58:18Z

Hmm well sorry for bringing up semver, because MkDocs doesn't actually follow it directly. It's more like "anything goes"."no major breaking changes"."no obvious breaking changes"

Anyway, from what you described (and I'm still not fully clear on it), seems like something could reasonably break, so then it's more like 1.3.0.
And to be fully clear again, it is fully out of the question to push it to 2.0.0, I was only considering 1.2.3 as the other option.

And thanks for suggesting a compatibility option, but uhh let's just rule that out entirely due to the big complexity.

Sparticuz · 2021-09-29T17:36:43Z

Modernizr got removed, which removes a bunch of css classes that get added by the modernizr javascript. If a user was using those css classes, it would be a breaking change.

To explain a little further, modernizr adds a bunch of css classes to the (html or body tag, I forget which). This allows a developer to target a specific browser capability. For example:

.no-cssgradients .header {
  background: url("images/glossybutton.png");
}

.cssgradients .header {
  background-image: linear-gradient(cornflowerblue, rebeccapurple);
}

Since modernizer was removed, the .no-cssgradients and .cssgradients classes would be gone and the developers css would fail to apply as intended.

"anything goes"."no major breaking changes"."no obvious breaking changes"

Then yes, I agree with 1.3.0. I believe it's a big enough change to justify a minor version rather than a patch version.

oprypin · 2021-11-07T09:24:38Z

Thank you very much!!

marcelstoer · 2021-11-07T09:41:46Z

@oprypin

I'm just thinking how we can involve more people to try out this theme

You could have tagged the people who created the issues this MR fixes (I am one of them). That way I'd have found out and could have helped testing before it was merged.

@Sparticuz thank you so much for all the hard work!

Looking forward to the next release.

oprypin · 2021-11-07T09:42:51Z

Well it's not a problem to try it out now, certainly please do :)

ajthealchemist · 2021-12-06T09:53:28Z

Example of logo image, as well as style_nav_header_background

Hi. Why is this not working for me despite including the following lines in my mkdocs.yml file? The logo is in docs/img.

theme:  
   name: readthedocs
   logo: img/logo.png

This is my mkdocs version:

mkdocs, version 1.2.3 from /home/username/.local/lib/python3.8/site-packages/mkdocs (Python 3.8)

Sparticuz · 2021-12-07T16:54:24Z

This is my mkdocs version:

mkdocs, version 1.2.3 from /home/username/.local/lib/python3.8/site-packages/mkdocs (Python 3.8)

I don't believe this PR has been released yet. Should be in 1.3

ajthealchemist · 2021-12-09T04:10:13Z

I don't believe this PR has been released yet. Should be in 1.3

I see. Thanks for the clarification. I thought this feature was already released. That being said, is there a tentative release schedule? Can't wait for this feature! <3

oprypin · 2021-12-09T07:55:02Z

Sorry, there is no tentative release schedule.

Sparticuz added 14 commits September 23, 2021 08:50

Upgrade to 0.4.2

d194545

readthedocs/sphinx_rtd_theme@0.4.1...0.4.2

Upgrade to 0.4.3

506a3f0

readthedocs/sphinx_rtd_theme@0.4.2...0.4.3

Fix comment

7d5fe5e

Fix test for style_nav_header_background config option

f90e095

Update rtd theme link

6a1faaa

Upgrade to 0.5.0rc1

5ecf25f

Add in writer-html5 class

ee76493

Upgrade to 0.5.0rc2

c865b83

Upgrade to 0.5.0

51b17ea

Adjust linting to use or exclude the correct rtd files

342417f

Upgrade to 0.5.1

d119336

Upgrade to 1.0.0rc1

82c4764

Upgrade to 1.0.0

641288b

Update messages.pot

2dc3c66

Sparticuz added 3 commits September 23, 2021 15:07

Upgrade to jQuery 3.6.0

859cd65

This was moved to a package.json in upstream for version 0.5.0rc1, then upgraded to 3.6.0 in 0.5.2

Fix test for anonymize_ip

1f6ecde

Add logo support

3cf33d9

Update messages.pot

3952594

Sparticuz marked this pull request as ready for review September 23, 2021 19:57

Add support for displaying a version

4c66a1a

oprypin reviewed Sep 26, 2021

View reviewed changes

Sparticuz added 2 commits September 27, 2021 11:04

Remove style_nav_header_background

9e7b681

This is being done in favor of using a css setting for `.wy-side-nav-search` Update config_tests.py

This translation msgid should be Mobile navigation menu

fbf4aa8

oprypin approved these changes Sep 27, 2021

View reviewed changes

oprypin reviewed Sep 27, 2021

View reviewed changes

mkdocs/themes/readthedocs/mkdocs_theme.yml Outdated Show resolved Hide resolved

Clarify logo variable

864a666

Update documentation for the readthedocs theme

2801ba2

Merge branch 'mkdocs:master' into master

8b99324

oprypin mentioned this pull request Oct 10, 2021

#2467 - Upgrade JQuery Versions #2468

Closed

oprypin added this to the 1.3.0 milestone Oct 26, 2021

oprypin merged commit fa52d3a into mkdocs:master Nov 7, 2021

oprypin mentioned this pull request Nov 16, 2021

Provide german locale file for readthedocs theme #2633

Merged

oprypin mentioned this pull request Dec 3, 2021

Translations for readthedocs theme are outdated #2695

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade readthedocs theme to v1.0.0 #2585

Upgrade readthedocs theme to v1.0.0 #2585

Sparticuz commented Sep 23, 2021 •

edited

Sparticuz commented Sep 23, 2021 •

edited

Sparticuz commented Sep 23, 2021

Sparticuz commented Sep 23, 2021

oprypin left a comment

oprypin commented Sep 27, 2021

Sparticuz commented Sep 27, 2021

pawamoy commented Sep 27, 2021 •

edited

oprypin commented Sep 27, 2021

oprypin commented Sep 27, 2021

Sparticuz commented Sep 28, 2021

oprypin commented Sep 28, 2021

Sparticuz commented Sep 28, 2021

oprypin commented Sep 28, 2021

Sparticuz commented Sep 29, 2021

oprypin commented Sep 29, 2021

Sparticuz commented Sep 29, 2021 •

edited by oprypin

oprypin commented Nov 7, 2021

marcelstoer commented Nov 7, 2021 •

edited

oprypin commented Nov 7, 2021

ajthealchemist commented Dec 6, 2021 •

edited

Sparticuz commented Dec 7, 2021

ajthealchemist commented Dec 9, 2021

oprypin commented Dec 9, 2021

Upgrade readthedocs theme to v1.0.0 #2585

Upgrade readthedocs theme to v1.0.0 #2585

Conversation

Sparticuz commented Sep 23, 2021 • edited

Sparticuz commented Sep 23, 2021 • edited

Sparticuz commented Sep 23, 2021

Sparticuz commented Sep 23, 2021

oprypin left a comment

Choose a reason for hiding this comment

oprypin commented Sep 27, 2021

Sparticuz commented Sep 27, 2021

pawamoy commented Sep 27, 2021 • edited

oprypin commented Sep 27, 2021

oprypin commented Sep 27, 2021

Sparticuz commented Sep 28, 2021

oprypin commented Sep 28, 2021

Sparticuz commented Sep 28, 2021

oprypin commented Sep 28, 2021

Sparticuz commented Sep 29, 2021

oprypin commented Sep 29, 2021

Sparticuz commented Sep 29, 2021 • edited by oprypin

oprypin commented Nov 7, 2021

marcelstoer commented Nov 7, 2021 • edited

oprypin commented Nov 7, 2021

ajthealchemist commented Dec 6, 2021 • edited

Sparticuz commented Dec 7, 2021

ajthealchemist commented Dec 9, 2021

oprypin commented Dec 9, 2021

Sparticuz commented Sep 23, 2021 •

edited

Sparticuz commented Sep 23, 2021 •

edited

pawamoy commented Sep 27, 2021 •

edited

Sparticuz commented Sep 29, 2021 •

edited by oprypin

marcelstoer commented Nov 7, 2021 •

edited

ajthealchemist commented Dec 6, 2021 •

edited