jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.51k stars 4.02k forks source link

Update dayjs locale when changing app language #12887

Closed medamineziraoui closed 3 years ago

medamineziraoui commented 3 years ago
Overview of the feature request

Currently when changing the language from application, the locale is not updated. it will keep the default locale of days. that need to change.

Motivation for or Use Case

Locale dayjs need to be updated according to the use language selection.

Related issues or PR

https://github.com/jhipster/generator-jhipster/issues/12575

medamineziraoui commented 3 years ago

@FabienEssid and myself started looking on how to add the improvement.

atomfrede commented 3 years ago

A confirm the issue for angular. ~It works for vue.~ I was wrong, it also doesn't work for vue.

I think for vue it can be added to translation.service.ts in refreshTranslation. Need to check for angular/react

It works for vue by extending translation.service.ts as follows:

import axios from 'axios';
import dayjs from 'dayjs';
import VueI18n from 'vue-i18n';
import { Store } from 'vuex';
import { BUILD_TIMESTAMP } from '@/constants';

export default class TranslationService {
  private store: Store<{}>;
  private i18n: VueI18n;

  constructor(store: Store<{}>, i18n: VueI18n) {
    this.store = store;
    this.i18n = i18n;
  }

  public refreshTranslation(newLanguage: string) {
    let currentLanguage = this.store.getters.currentLanguage;
    currentLanguage = newLanguage ? newLanguage : 'en';
    if (this.i18n && !this.i18n.messages[currentLanguage]) {
      this.i18n.setLocaleMessage(currentLanguage, {});
      dayjs.locale(currentLanguage);
      axios.get(`i18n/${currentLanguage}.json?buildTimestamp=${BUILD_TIMESTAMP}`).then(res => {
        if (res.data) {
          this.i18n.setLocaleMessage(currentLanguage, res.data);
          this.i18n.locale = currentLanguage;
          this.store.commit('currentLanguage', currentLanguage);
        }
      });
    } else if (this.i18n) {
      this.i18n.locale = currentLanguage;
      dayjs.locale(currentLanguage);
      this.store.commit('currentLanguage', currentLanguage);
    }
  }
}

@medamineziraoui Would you like to do a PR? If you need more help don't hesitate to reach out here.

medamineziraoui commented 3 years ago

Hello @atomfrede yes i would love to, this would be my first contribution if done successfully so definitely would not hesitate to ask if I need help.

atomfrede commented 3 years ago

@medamineziraoui Awesome! I think for angular you need to look here and for react it could be done in the locale reducer/store

medamineziraoui commented 3 years ago

@atomfrede Thank you

FabienEssid commented 3 years ago

@medamineziraoui is currently on holidays. We should work on this issue when he'll be back next week :slightly_smiling_face:

medamineziraoui commented 3 years ago

Hello im starting working on this issue today, i will keep you updated with my progress :slightly_smiling_face:

medamineziraoui commented 3 years ago

export default (state: LocaleState = initialState, action): LocaleState => { switch (action.type) { case ACTION_TYPES.SET_LOCALE: { const currentLocale = action.locale; if (state.currentLocale !== currentLocale) { dayjs.locale(currentLocale); // <====== updated this dayjs locale before setting state in the TranslatorContext TranslatorContext.setLocale(currentLocale);


What do think about updating React this way to fixe the dayjs locale?
- [x] Angular: i tried starting the project but i got an error 
![image](https://user-images.githubusercontent.com/57661415/100020684-80b1eb00-2de0-11eb-8cfb-8c7a2fed44c9.png)
I will look it up tomorrow
medamineziraoui commented 3 years ago
DanielFran commented 3 years ago

@medamineziraoui You need to checkout jhipster-bom project and build it locally. There is no breaking change between released version 0.1.0 and current master (7.0.0-SNAPSHOT) of jhipster-bom project until now... But better to always compile it locally.

medamineziraoui commented 3 years ago

Thank you for your answer @DanielFran, are you talking about this repo ? https://github.com/jhipster/jhipster-bom Is there anything else I need to do ( any change in configuration for generated project ?)

DanielFran commented 3 years ago

Yes, it is this repo. No changes required on generator side... Please follow the instructions how to build jhipster-bom project in the readme.

medamineziraoui commented 3 years ago

Ok thank you, i will test this tomorrow :slightly_smiling_face:

kaidohallik commented 3 years ago
  • Angular: i tried starting the project but i got an error

Try to delete node_modules folder and package-lock.json file and run npm install again, this can solve this error.

medamineziraoui commented 3 years ago

Thank you @kaidohallik i will try this later this day and i will keep you updated :slightly_smiling_face:

medamineziraoui commented 3 years ago

I tried both solution for jhipster-bom and node_modules and works fine ( needed to upgrade node version too for angular ), i got another issue when trying to link the generator-jhipster image i will check it out and continue tomorrow

medamineziraoui commented 3 years ago

Never mind i upgraded the node and it works now image

DanielFran commented 3 years ago

@medamineziraoui Do you still have issues with your setup?

medamineziraoui commented 3 years ago

Hello no i don't have setups issues thx to your advice guys so thank you, i wasn't able to work fixe the issue those pass days but i will this weekend.

github-actions[bot] commented 3 years ago

This issue is stale because it has been open 30 days with no activity. Our core developers tend to be more verbose on denying. If there is no negative comment, possibly this feature will be accepted. We are accepting PRs :smiley:. Comment or this will be closed in 7 days

atomfrede commented 3 years ago

Keep it open

atomfrede commented 3 years ago

@medamineziraoui Whats your status on this? If you like I can pick up where you left and finish it.

medamineziraoui commented 3 years ago

Hello @atomfrede sorry for being disconnected for this long :sweat_smile: , but if you let me I would like to continue work on this issue this weekend, i did test the fixes on monday and it s worked for

I will send a Fix this weekend.

atomfrede commented 3 years ago

Sure, go ahead, looking forward to the PR!

medamineziraoui commented 3 years ago

Thank you :smile:

medamineziraoui commented 3 years ago

Hello @atomfrede I added Pull Request , i tried to add test for react but couldn't make it work,

 it('should current locale state be coherent with dayjs locale', () => {
    const localeState = locale({ currentLocale: 'en' }, { type: ACTION_TYPES.SET_LOCALE, locale: 'es' });
    expect(dayjs.locale()).toEqual(localeState?.currentLocale);
  });

Maybe locale method is async This is the first time I try to write test with jest so could be if i did something wrong.

I ran test with vuejs but i got 2 failed tests, i don t believe they are related to my changes.

image

medamineziraoui commented 3 years ago

Thank you for letting me finish the PR, and i apologize for taking long to finish a simple fixe

atomfrede commented 3 years ago

No need to apologize! We are all doing this in our freetime. Thanks for contributing.