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

i18n capabilities added #775

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lgriffin
Copy link
Contributor

Please check if the PR fulfills these requirements

  • The commit message is sensible and easily understood
  • Tests for the changes have been added (for bug fixes / features where relevant)
  • Docs have been added / updated (for bug fixes / features where relevant)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

This brings a new feature that addresses #674. Specifically it brings in an extensible approach to i18n and the working example I have allows us to quickly change the text on several key components in the screen. I am pausing now for feedback as there are other components that would need updating but the PR is mature enough that you can see the workings of it on a subset of the screen. I used Google Translate to try and get some realistic language capabilities in place. I wrote a mini guide for extending it as well. Here are some to:dos that I have identified:

  • Update the remaining page components, some of these look to be in a different project so I have to figure out how to grab and extend in that sense
  • Get i18n icons that are open source to make the pop out menu I added a bit more usable
  • Get actual native speakers to translate, I'm sure my google translate efforts leave a lot to be desired

@jouwdan and @edeleastar can you both take a look at this, check am I on the right path and give me any feedback on the layout I picked for components and naming conventions. I will chip away at this and rebase it later when I get time to add more components. The initial uplift is there though, the solution is scalable and shouldn't be too much challenge to modify the remaining svelte pages.

What commits does this PR relate to?

Thank you for your contribution

We hope you stay around and connect with our growing community!

Added some capabilities for i18n to extend
@edeleastar
Copy link
Contributor

edeleastar commented Jun 18, 2024

Thanks @lgriffin

very interesting - and works very well.

The approach seems sound - although I think we would aim for it just on the home page, and not throughout the app just yet. This might involve moving the current lib folder into the routes/(home). We can isolate it there first, promoting it higher if we decide to implement i8n more widely.

Also - PR to development instead of main?

Eamonn

@lgriffin
Copy link
Contributor Author

Apologies my email filter ate the notification for this :) What I will do is revist this next week hopefully and try round out the final few components on the home page and rebase against dev branch.

@edeleastar
Copy link
Contributor

Hi @lgriffin. No rush, I left some notes on the pdf search suggestion in case you may have missed that one

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