Skip to content

Conversation

@speco29
Copy link
Contributor

@speco29 speco29 commented Jun 28, 2025

Screen.Recording.2025-06-30.135547.mp4

@OriolAbril I will be keeping this as final 404 page, let me know if there is anything to change. This 404 page required to create an /es folder inside DISCOVER/build/html containing every html page.

@OriolAbril
Copy link
Contributor

The video seems to be switching from 404.html to es/404.html. We should test this from your fork and see how it behaves on github when landing on non-existing pages (instead of landing at the 404.html one). Have you published the website with this 404 on your fork? Could you do it and share the link here?

@speco29
Copy link
Contributor Author

speco29 commented Jul 2, 2025

The video seems to be switching from 404.html to es/404.html. We should test this from your fork and see how it behaves on github when landing on non-existing pages (instead of landing at the 404.html one). Have you published the website with this 404 on your fork? Could you do it and share the link here?

Actually, when I am trying to deploy this, an error is showing regarding some permission, so its not getting deployed.
But on server, it works well for broken links as well:

Screen.Recording.2025-07-03.003828.mp4

@OriolAbril
Copy link
Contributor

The part I don't understand is the 404 and es/404 pages. We have the 404.html page in the DISCOVER folder. Then we have the same page also as html in locales/es? We also build a placeholder for the website in spanish and copy the main 404.html in it (not the one in locales). Then when we change the language we are actually changing from 404.html to es/404.html, which seems to be what we saw in the other test doesn't work? But also, if we do that and it works, why do we need the javascript for? Can't we have a static list/dropdown pointing to the different lang/404.html pages with each of these also being static?

@speco29
Copy link
Contributor Author

speco29 commented Jul 3, 2025

The part I don't understand is the 404 and es/404 pages. We have the 404.html page in the DISCOVER folder. Then we have the same page also as html in locales/es? We also build a placeholder for the website in spanish and copy the main 404.html in it (not the one in locales). Then when we change the language we are actually changing from 404.html to es/404.html, which seems to be what we saw in the other test doesn't work? But also, if we do that and it works, why do we need the javascript for? Can't we have a static list/dropdown pointing to the different lang/404.html pages with each of these also being static?

I've implemented a multilingual 404 page using a single custom-designed 404.html file that includes translations for English and Spanish. This file is placed in the root of the site and also copied into the /es/ folder because GitHub Pages serves 404 pages based on the folder where the broken link occurs. For example, a broken link under /es/ will only trigger /es/404.html, not the root /404.html.

Although we could create separate static 404 pages for each language, I chose to use JavaScript to dynamically detect the language (based on the URL or referrer) and update the text accordingly. This allows us to maintain just one file instead of duplicating content for each language. The language switcher dropdown also uses JS to redirect users to the appropriate version (/404.html for English, /es/404.html for Spanish).

If you prefer static, language-specific 404s, we can skip JS — but we’ll need to maintain multiple files. Each file would be hardcoded in its own language.


const pathLang = location.pathname.split("/")[1];
const refLang = new URL(document.referrer || "", location.origin).pathname.split("/")[1];
const lang = translations[pathLang] ? pathLang : (translations[refLang] ? refLang : "en");
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be the reason it doesn't work on the root 404 page? We set the language through the switcher but the language defined from the path takes priority?

Comment on lines 152 to 159
document.getElementById("language-switcher").addEventListener("change", function () {
const selectedLang = this.value;
if (selectedLang === "en") {
window.location.href = `/404.html`;
} else {
window.location.href = `/${selectedLang}/404.html`;
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

or maybe this isn't updating the language related constants? not even sure it is possible though (0 javascript knowledge on my end)

@speco29
Copy link
Contributor Author

speco29 commented Jul 10, 2025

@OriolAbril I have published the website on my fork, here's the link: https://speco29.github.io/DISCOVER-Cookbook/in.html
Now there is only one 404.html page that is global, no more going to different folders. Please review it as per your convenience.

@OriolAbril
Copy link
Contributor

The switching language while maintaining the broken url is working which is great.

Given there is a switcher, it is not strictly necessary (and might even be a good idea if it allows to simplify the code significantly). The part that is not working now is the default language as taken from the URL: https://speco29.github.io/DISCOVER-Cookbook/es/bad.html shows in English.

From the little I understand of the code, I think it has to do with what is expected of the URLs when splitting it through / which is hopefully an easy fix. If so I think we should add some comments to explain a bit what is happening so it is clear it needs to be updated if the base url had to change (say from https://discover-cookbook.numfocus.org to https://numfocus.org/discover-cookbook. Also note we are going to have baseurl/version/language/page.html once the full matrix of version/languages is working

@speco29
Copy link
Contributor Author

speco29 commented Jul 14, 2025

The part that is not working now is the default language as taken from the URL: https://speco29.github.io/DISCOVER-Cookbook/es/bad.html shows in English.

But it shows in spanish in my laptop, when I clicked on that link, by default it showed spanish:

Screen.Recording.2025-07-14.223841.mp4

Altthough when I changed it into english, the folder still remains the same /es/bad.html.

@OriolAbril
Copy link
Contributor

But it shows in spanish in my laptop, when I clicked on that link, by default it showed spanish:

It also works on my phone, but still getting English on my laptop. I guess it is an issue with the browser on my laptop. As I said, I can change manually so not a problem.

Altthough when I changed it into english, the folder still remains the same /es/bad.html.

That is what we want to happen in the 404 page.

@speco29
Copy link
Contributor Author

speco29 commented Jul 15, 2025

That is what we want to happen in the 404 page.

@OriolAbril Is there something that I should change/add in this page?

@netlify
Copy link

netlify bot commented Jul 26, 2025

Deploy Preview for stupendous-kringle-a86e81 ready!

Name Link
🔨 Latest commit e8f9c22
🔍 Latest deploy log https://app.netlify.com/projects/stupendous-kringle-a86e81/deploys/688e27257aa54700088b2d6a
😎 Deploy Preview https://deploy-preview-344--stupendous-kringle-a86e81.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@speco29 speco29 marked this pull request as ready for review July 26, 2025 19:06
Comment on lines 25 to 30
- name: Copy global 404.html
run: |
cp DISCOVER/404.html DISCOVER/_build/html/404.html
Copy link
Contributor

Choose a reason for hiding this comment

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

the build website script already does this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll remove it then

Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be part of this PR but a different one. Not sure why this file was missing form the toc but it should definitely be added, just in a different PR

@OriolAbril
Copy link
Contributor

OriolAbril commented Jul 29, 2025

This PR we can keep with main as target as it won't affect any of the existing content, only the 404 page. We have to keep this in mind as we work on next so the "back to home" continues working as expected and points to the preferred version of the website

@speco29
Copy link
Contributor Author

speco29 commented Jul 30, 2025

@OriolAbril I have made few changes, please you review it as per your convenience and let me know about it;)

Copy link
Contributor

Choose a reason for hiding this comment

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

also undo the rename of the file as it is done in #365

@OriolAbril OriolAbril merged commit 96c2346 into numfocus:main Aug 2, 2025
4 checks passed
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.

2 participants