search

canonical / sphinx-docs-starter-pack

A documentation starter-pack
https://canonical-starter-pack.readthedocs-hosted.com/
Other
13 stars 35 forks source link

feat: remove notfound.extension #180

Closed dviererbe closed 7 months ago

dviererbe commented 7 months ago

ReadTheDocs will automatically serve the content of /404.html or 404/index.html if a page could not be found. Therefore I replaced this extension with a 404 page written in reStructuredText.

This will also fix issues #138 and #139.

See https://docs.readthedocs.io/en/stable/reference/404-not-found.html

dviererbe commented 7 months ago

Note: This is how the new 404 page looks like:

Screenshot from 2024-01-25 16-28-56

dviererbe commented 7 months ago

Note: I reused the text from the notfound.extension configuration

ru-fu commented 7 months ago

This will need some testing on whether it works both with RTD URLs (for example https://canonical-microcloud.readthedocs-hosted.com/en/latest/ ) and documentation.ubuntu.com URLs (for example https://documentation.ubuntu.com/lxd/en/latest/ ). The current solution works with both (just not with the RTD preview).

Also, I remember @s-makin and @pmatulis being opposed to a separate 404 page (see https://github.com/canonical/sphinx-docs-starter-pack/pull/55). Any opinions from your side?

I really like the look (the image) though. ;)

pmatulis commented 7 months ago

What does the new page look like in dark mode?

Disregard. Tested this and it looks okay.

I'm fine with this then.

ru-fu commented 7 months ago

Did anybody test whether this will actually work in different setups?

pmatulis commented 7 months ago

@ru-fu What do you mean by setups?

ru-fu commented 7 months ago

@ru-fu What do you mean by setups?

What I wrote above:

This will need some testing on whether it works both with RTD URLs (for example https://canonical-microcloud.readthedocs-hosted.com/en/latest/ ) and documentation.ubuntu.com URLs (for example https://documentation.ubuntu.com/lxd/en/latest/ ). The current solution works with both (just not with the RTD preview).

It was quite tricky to get the original solution working in both scenarios.

pmatulis commented 7 months ago

It was quite tricky to get the original solution working in both scenarios.

I'm sorry. No, I didn't test that. Should we revert?

ru-fu commented 7 months ago

I'm sorry. No, I didn't test that. Should we revert?

I'll try to find time for testing next week. No need to revert if it works ...