-
-
Notifications
You must be signed in to change notification settings - Fork 248
Adding 404 page for the website #340
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
Changes from all commits
d46b02e
4a462ca
ec57958
d24ac21
7420810
a0dd90a
7d1ce7e
1f519a8
5962c5e
78058a6
60f7866
95e1852
589289a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| name: Deploy Jupyter Book to GitHub Pages | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - options-doc | ||
|
|
||
| permissions: | ||
| contents: write | ||
|
|
||
| jobs: | ||
| deploy: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.10" | ||
|
|
||
| - name: Install Jupyter Book and sphinx-tags | ||
| run: pip install -U jupyter-book sphinx-tags | ||
|
|
||
| - name: Build Jupyter Book | ||
| run: jupyter-book build DISCOVER/ | ||
|
|
||
| - name: Deploy to gh-pages branch | ||
| uses: peaceiris/actions-gh-pages@v4 | ||
| with: | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| publish_dir: DISCOVER/_build/html | ||
| publish_branch: gh-pages |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you deployed this to your preview? I would like to test how it handles a variety of broken urls, getting the 404 page to use the website theme consistently is a bit tricky so we might need to use https://sphinx-notfound-page.readthedocs.io/en/latest/ That being said, I am not sure having the 404 page use the theme will be the best user experience (for example, I don't think the switchers will work from the 404 page)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, I deployed it. It goes with the theme and looks something like this:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can have a raw html page "outside the theme" yes, no need for it to be markdown first. The 404 page will also be global to versions and a future version might change the theme too |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| # 🚫 <span id="title">404 - Page Not Found</span> | ||
|
|
||
| <p id="message">Oops! It looks like the page you're looking for has wandered off.</p> | ||
|
|
||
| <p id="suggestion">Maybe it was deleted, renamed, or never existed in the first place.</p> | ||
|
|
||
| --- | ||
|
|
||
| ### <span id="what-do">What can you do?</span> | ||
|
|
||
| - <a href="./intro.html" id="home">Return to the homepage</a> | ||
| - <a href="https://github.com/numfocus/DISCOVER-Cookbook/issues" id="report">Raise an issue</a> | ||
|
|
||
| --- | ||
|
|
||
| <p id="footer">If you think this is an error, let us know by creating an issue.</p> | ||
|
|
||
| <script> | ||
| const translations = { | ||
| en: { | ||
| title: "404 - Page Not Found", | ||
| message: "Oops! It looks like the page you're looking for has wandered off.", | ||
| suggestion: "Maybe it was deleted, renamed, or never existed in the first place.", | ||
| whatDo: "What can you do?", | ||
| home: "Return to the homepage", | ||
| report: "Raise an issue", | ||
| footer: "If you think this is an error, let us know by creating an issue." | ||
| }, | ||
| es: { | ||
| title: "Página No Encontrada", | ||
| message: "¡Ups! Parece que la página que buscas se ha extraviado.", | ||
| suggestion: "Tal vez fue eliminada, renombrada o nunca existió.", | ||
| whatDo: "¿Qué puedes hacer?", | ||
| home: "Volver a la página principal", | ||
| report: "Abre un issue", | ||
| footer: "Si crees que esto es un error, infórmanos creando un issue." | ||
| } | ||
| // Add more translations as needed | ||
| }; | ||
|
|
||
| const lang = (navigator.language || "en").split("-")[0]; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure we want this behaviour. Maybe we could check the url the 404 is being generated for? Is that possible? The main issue I expect out of this. My computer and browser are in catalan. Big projects like linux distros or firefox have a catalan translation, but most places do not (the cookbook will probably be here). Now I don't care so much, but if both languages are available I often prefer spanish. If I understand the code correctly with this setup I would switch from browsing the Spanish translation to the english 404 I was originally thinking about having all the languages there all at once. But it won't be nice once languages start to grow.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think Instead of relying on Something like this: // Determine language from URL path — fallback to 'en'
const urlLang = location.pathname.split("/")[1] || "en";
const lang = translations[urlLang] ? urlLang : "en";
// Update content with appropriate language
document.getElementById("title").textContent = translations[lang].title;
document.getElementById("message").textContent = translations[lang].message;
document.getElementById("suggestion").textContent = translations[lang].suggestion;
document.getElementById("what-do").textContent = translations[lang].whatDo;
document.getElementById("home").textContent = translations[lang].home;
document.getElementById("report").textContent = translations[lang].report;
document.getElementById("footer").textContent = translations[lang].footer;Also since the 404 stands apart from our standard navigation, we could offer a subtle dropdown to switch languages. <select id="language-switcher">
<option value="en">English</option>
<option value="es">Español</option>
<option value="fr">Français</option>
</select>then on addition: document.getElementById("language-switcher").addEventListener("change", function () {
const lang = this.value;
window.location.href = `/${lang}/404.html`;
});This allows users to switch to the correct translated 404 page even if they’ve landed on the wrong one — much better for experience and accessibility.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sounds better, especially the option to allow users to choose. It is basically impossible to accurately guess/predict which language will someone request when reading a page. Url based default+allow choosing will be a good user experience I think |
||
| const t = translations[lang] || translations["en"]; | ||
|
|
||
| document.getElementById("title").textContent = t.title; | ||
| document.getElementById("message").textContent = t.message; | ||
| document.getElementById("suggestion").textContent = t.suggestion; | ||
| document.getElementById("what-do").textContent = t.whatDo; | ||
| document.getElementById("home").textContent = t.home; | ||
| document.getElementById("report").textContent = t.report; | ||
| document.getElementById("footer").textContent = t.footer; | ||
| </script> | ||
This file was deleted.

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically duplicating https://github.com/numfocus/DISCOVER-Cookbook/blob/main/.github/workflows/build-gh-pages.yml. I don't think we want this workflow at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will delete this file deploy.yml also I am thinking to close this pull request and start a new pull request that has everything in a correct way, I opened this as a draft PR because I knew it would conatin some mistakes and changes would be required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is easier do not hesitate to open a different PR, but if it is only to keep the "commit history clean" I personally don't care at all, and it should be possible to merge with "squash and merge" so the whole PR becomes a single commit and no track of the PR commits remain if that were to worry you.