Skip to content

Conversation

MomdAli
Copy link
Collaborator

@MomdAli MomdAli commented Sep 5, 2025

Locale management and formatting improvements:

  • Added setSystemLocale, getSystemLocale, and applySystemLocaleOnDate methods to the DateFormatter utility class, allowing centralized management of the application's locale and consistent formatting of dates in DatePicker components. (DateFormatter.java) [1] [2]
  • On application startup, the system locale is now set in DateFormatter, and the default locale is explicitly set to English to ensure predictable behavior. (App.java) [1] [2]

* using system locale for date/times
@MomdAli MomdAli requested a review from Death111 September 5, 2025 11:05
@MomdAli MomdAli self-assigned this Sep 5, 2025
@MomdAli MomdAli marked this pull request as ready for review September 5, 2025 12:10
@MomdAli
Copy link
Collaborator Author

MomdAli commented Sep 5, 2025

@CodiumAI-Agent /review

@CodiumAI-Agent
Copy link

CodiumAI-Agent commented Sep 5, 2025

PR Reviewer Guide 🔍

(Review updated until commit c9cc12b)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Locale Initialization Order

The app sets DateFormatter's system locale to the current default, then immediately overrides the JVM default to English. Verify that components created before/after this sequence use the intended locale and that English default does not unintentionally override user expectations elsewhere.

DateFormatter.setSystemLocale(Locale.getDefault());
Locale.setDefault(Locale.ENGLISH);
final DefaultExceptionHandler defaultExceptionHandler = new DefaultExceptionHandler();
Thread Safety

The static mutable field systemLocale is updated via setSystemLocale without synchronization. If accessed or changed from multiple threads (UI vs. background), consider making it volatile or using synchronized access to avoid race conditions.

public static void setSystemLocale(Locale locale) {
   systemLocale = locale;
}

public static Locale getSystemLocale() {
   return systemLocale;
}
Fragile UI Locale Assertions

The test checks for localized strings like "Montag" and "06.2024". This may be brittle across JDK vendors or OS settings. Consider asserting via formatter pattern or parsing round-trip rather than string contains.

@Test
void applySystemLocaleShouldSetDatePickerLocale() {
   // GIVEN
   Locale defaultLocale = Locale.getDefault();
   Locale.setDefault(Locale.ENGLISH);
   DateFormatter.setSystemLocale(Locale.GERMAN);
   DatePicker datePicker = new DatePicker();
   LocalDate date = LocalDate.of(2024, 6, 3);

   // WHEN
   DateFormatter.applySystemLocaleOnDate(datePicker);
   datePicker.setValue(date);

   // THEN
   String shown = datePicker.getEditor().getText();
   assertTrue(shown.contains("Montag"));
   assertTrue(shown.contains("06.2024"));

   Locale.setDefault(defaultLocale);
}

@MomdAli MomdAli requested a review from Death111 September 15, 2025 08:51
@Death111
Copy link
Collaborator

Death111 commented Oct 2, 2025

@MomdAli I was using it the last weeks and I did not notice issues - only annoying thing is the calendar view that sunday is the first day of the week. can this be changed / also localized somehow?
grafik

@CodiumAI-Agent
Copy link

Persistent review updated to latest commit c9cc12b

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.

3 participants