Skip to content

Conversation

@alf0ns3
Copy link
Contributor

@alf0ns3 alf0ns3 commented Jan 4, 2026

  • i18n used to localize all the frontend strings
  • fr localization
  • de localization

@alf0ns3 alf0ns3 requested a review from 0x46616c6b as a code owner January 4, 2026 14:03
Copy link
Member

@0x46616c6b 0x46616c6b left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution! I highly appreciate it. Could you please check to ensure the translations are working with the test setup? I could add the German translations afterwards.

@alf0ns3
Copy link
Contributor Author

alf0ns3 commented Jan 4, 2026

@0x46616c6b yes sure, I'm working with React for the first time so let me know if creating a new test-utils.tsx file is not the correct way to do.

@alf0ns3
Copy link
Contributor Author

alf0ns3 commented Jan 4, 2026

Also, locally I have an error running npm run coverage:

TypeError: localStorage.getItem is not a function

I made all the tests working by adding this snippet on test-utils.tsx file:

// Mock localStorage for tests
Object.defineProperty(window, 'localStorage', {
  value: {
    getItem: vi.fn(() => null),
    setItem: vi.fn(() => {}),
    removeItem: vi.fn(() => {}),
    clear: vi.fn(() => {}),
  },
  writable: true,
})

but I don't have a local Ticker instance running and it seems to don't be linked to the coded I added, so that I didn't commited it yet.

@0x46616c6b
Copy link
Member

Also, locally I have an error running npm run coverage:

TypeError: localStorage.getItem is not a function

I made all the tests working by adding this snippet on test-utils.tsx file:

// Mock localStorage for tests
Object.defineProperty(window, 'localStorage', {
  value: {
    getItem: vi.fn(() => null),
    setItem: vi.fn(() => {}),
    removeItem: vi.fn(() => {}),
    clear: vi.fn(() => {}),
  },
  writable: true,
})

but I don't have a local Ticker instance running and it seems to don't be linked to the coded I added, so that I didn't commited it yet.

You can use our Ticker API, we provide a Ticker for local development:

TICKER_API_URL="https://ticker-api.systemli.org/v1"

@0x46616c6b
Copy link
Member

Is it okay if I make some simplifications based on your code changes?

@alf0ns3
Copy link
Contributor Author

alf0ns3 commented Jan 4, 2026

Is it okay if I make some simplifications based on your code changes?

Yeah sure!

@0x46616c6b
Copy link
Member

Hmm, I can't push to your fork. But here what I would do:

  • remove test-utils.tsx (not necessary)
  • revert the changes on the test files
  • add the following to the vitest-setup.ts
import { initReactI18next } from 'react-i18next'
import i18n from 'i18next'
import en from './src/i18n/locales/en.json'

i18n.use(initReactI18next).init({
  resources: { en: { translation: en } },
  lng: 'en',
  fallbackLng: 'en',
  interpolation: { escapeValue: false },
})

@alf0ns3
Copy link
Contributor Author

alf0ns3 commented Jan 4, 2026

Also, locally I have an error running npm run coverage:

TypeError: localStorage.getItem is not a function

I made all the tests working by adding this snippet on test-utils.tsx file:

// Mock localStorage for tests
Object.defineProperty(window, 'localStorage', {
  value: {
    getItem: vi.fn(() => null),
    setItem: vi.fn(() => {}),
    removeItem: vi.fn(() => {}),
    clear: vi.fn(() => {}),
  },
  writable: true,
})

but I don't have a local Ticker instance running and it seems to don't be linked to the coded I added, so that I didn't commited it yet.

You can use our Ticker API, we provide a Ticker for local development:

TICKER_API_URL="https://ticker-api.systemli.org/v1"

I still have the same error TypeError: localStorage.getItem is not a function when I run locally npm run test, even with the TICKER_API_URL correctly configured (I can access the test messages from within http://localhost:4000).

Do you have an idea to make it working accordingly?

@alf0ns3
Copy link
Contributor Author

alf0ns3 commented Jan 4, 2026

I wanted to update dayjs behavior to use the locale defined by the browser, but I didn't managed to make it working yet

@0x46616c6b
Copy link
Member

Which node version are you using? The CI and also on my machine with Node LTS/JOD (22) the tests are passing.

@alf0ns3
Copy link
Contributor Author

alf0ns3 commented Jan 4, 2026

Which node version are you using? The CI and also on my machine with Node LTS/JOD (22) the tests are passing.

node --version
v25.2.1

npm --version 
11.6.2

So I'll rollback to v22

@alf0ns3
Copy link
Contributor Author

alf0ns3 commented Jan 4, 2026

Hmm, I can't push to your fork. But here what I would do:

* remove `test-utils.tsx` (not necessary)

* revert the changes on the test files

* add the following to the `vitest-setup.ts`
import { initReactI18next } from 'react-i18next'
import i18n from 'i18next'
import en from './src/i18n/locales/en.json'

i18n.use(initReactI18next).init({
  resources: { en: { translation: en } },
  lng: 'en',
  fallbackLng: 'en',
  interpolation: { escapeValue: false },
})

I gave you an access to my fork if you want to push code directly there

@0x46616c6b
Copy link
Member

I use nvm and the .nvmrc file to manage the version for projects.

@0x46616c6b
Copy link
Member

The PR looks really good to me. You only want to make dayjs i18n aware as well, correct?

@alf0ns3
Copy link
Contributor Author

alf0ns3 commented Jan 4, 2026

I use nvm and the .nvmrc file to manage the version for projects.

thanks for the tip, it's working now

@alf0ns3
Copy link
Contributor Author

alf0ns3 commented Jan 4, 2026

The PR looks really good to me. You only want to make dayjs i18n aware as well, correct?

I managed to make dayjs locale working! e931d1c

This PR is ready to be merged now

@doobry-systemli
Copy link
Contributor

Really nice 😍 Maybe a small comment in the readme on how to update the localization JSON files (e.g. after introducing a new translation string) would be helpful.

@0x46616c6b 0x46616c6b added the enhancement New feature or request label Jan 6, 2026
@0x46616c6b 0x46616c6b changed the title Feature: Localization in fr and de ✨ Add support for I18n Jan 6, 2026
@0x46616c6b 0x46616c6b merged commit 9fca5c4 into systemli:main Jan 6, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants