Fight-details-ui#194
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enhances the fight details UI with internationalization support for date formatting and several UI improvements. The changes aim to make duration displays locale-aware and improve the user experience with better modal layouts and search capabilities.
Changes:
- Added locale-aware date formatting for fight durations and age differences
- Improved UI with responsive modal dialogs, rounded borders, and refined button styling
- Enhanced search functionality with diacritic-insensitive matching
- Refined weight difference display with 2 decimal precision
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/utils/date.utils.ts | New utility to get date-fns locale based on current i18n locale |
| client/src/utils/string.utils.ts | Updated age difference formatting to use locale-aware date-fns |
| client/src/components/tournament/analytics-modal.component.vue | Added locale support for total fight duration formatting |
| client/src/components/shared/matchup/matchup-details.component.vue | Improved weight difference precision and added rounded border styling |
| client/src/components/tournament/matchmaker-modal.component.vue | Added fullscreen mode for small screens |
| client/src/components/selector/boxer-selector.component.vue | Added diacritic-insensitive search matching |
| client/src/components/fight-card/fight-details-modal.component.vue | Changed navigation button styling from primary to secondary |
| const end = date1 < date2 ? date2 : date1 | ||
| const duration = intervalToDuration({ start, end }) | ||
| return formatDuration(duration, { format: ["years", "months", "days"] }) | ||
| return formatDuration(duration, { format: ["years", "months", "days"], locale: getDateFnsLocale() }) |
There was a problem hiding this comment.
The getDateFnsLocale() function is called from getAgeDifference(), which itself is called during template rendering. However, getDateFnsLocale() calls useI18n() which can only be invoked during the component setup phase. This will cause a runtime error.
The locale should be obtained during setup and either passed as a parameter to the utility function or the utility function should be refactored to not depend on Vue's composition API.
| import { useI18n } from "vue-i18n" | ||
| import { fr } from "date-fns/locale/fr" | ||
| import { enUS } from "date-fns/locale/en-US" | ||
| import { Locale as DateFnsLocale } from "date-fns" | ||
|
|
||
| /** | ||
| * Returns the date-fns locale object matching the current vue-i18n locale. | ||
| * Defaults to enUS if not matched. | ||
| */ | ||
| export function getDateFnsLocale(): DateFnsLocale { | ||
| // Use inside setup() or a component | ||
| const { locale } = useI18n() | ||
| const lang = typeof locale === "string" ? (locale as string).split("-")[0] : locale.value.split("-")[0] |
There was a problem hiding this comment.
The getDateFnsLocale() function calls useI18n() which can only be used inside Vue component setup functions or composable functions. This function is called from getAgeDifference() in string.utils.ts, which is a plain utility function that may be called outside of component context. This will cause a runtime error.
Consider making this a composable function that returns functions which have access to the i18n instance, similar to the pattern used in labels.utils.ts with useLabels(), or pass the locale as a parameter to the functions that need it.
| import { useI18n } from "vue-i18n" | |
| import { fr } from "date-fns/locale/fr" | |
| import { enUS } from "date-fns/locale/en-US" | |
| import { Locale as DateFnsLocale } from "date-fns" | |
| /** | |
| * Returns the date-fns locale object matching the current vue-i18n locale. | |
| * Defaults to enUS if not matched. | |
| */ | |
| export function getDateFnsLocale(): DateFnsLocale { | |
| // Use inside setup() or a component | |
| const { locale } = useI18n() | |
| const lang = typeof locale === "string" ? (locale as string).split("-")[0] : locale.value.split("-")[0] | |
| import { fr } from "date-fns/locale/fr" | |
| import { enUS } from "date-fns/locale/en-US" | |
| import { Locale as DateFnsLocale } from "date-fns" | |
| /** | |
| * Returns the date-fns locale object matching the provided locale. | |
| * Defaults to enUS if not matched. | |
| * | |
| * @param locale Optional locale, e.g. "en-US", "fr-FR", or an object with a `value` string property. | |
| */ | |
| export function getDateFnsLocale(locale?: string | { value: string }): DateFnsLocale { | |
| let rawLocale: string | undefined | |
| if (typeof locale === "string") { | |
| rawLocale = locale | |
| } else if (locale && typeof (locale as any).value === "string") { | |
| rawLocale = (locale as any).value | |
| } else if (typeof navigator !== "undefined" && typeof navigator.language === "string") { | |
| rawLocale = navigator.language | |
| } else { | |
| rawLocale = "en-US" | |
| } | |
| const lang = rawLocale.split("-")[0] |
| const dateFnsLocale = getDateFnsLocale() | ||
| return formatDuration(duration, { format: ["hours", "minutes", "seconds"], locale: dateFnsLocale }) |
There was a problem hiding this comment.
The getDateFnsLocale() function is called inside a computed property getter, but it calls useI18n() which can only be invoked during the component setup phase. This will cause a runtime error when the computed property is evaluated.
The locale should be obtained during setup and either passed as a parameter to utility functions or captured in a variable that can be safely accessed later.
…ails layout; add locale support for date-fns in string and date utilities
b7dd93a to
940b99f
Compare
No description provided.