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

[Bug Report][3.4.9] VDataTable i18n page options #18978

Closed
vincentauger opened this issue Jan 4, 2024 · 9 comments · Fixed by #18980 or Enterprise-CMCS/cmcs-eregulations#1307 · May be fixed by Shuunen/ging#200
Closed

[Bug Report][3.4.9] VDataTable i18n page options #18978

vincentauger opened this issue Jan 4, 2024 · 9 comments · Fixed by #18980 or Enterprise-CMCS/cmcs-eregulations#1307 · May be fixed by Shuunen/ging#200
Assignees
Labels
C: VDataTable VDatatable T: bug Functionality that does not work as intended/expected

Comments

@vincentauger
Copy link
Contributor

vincentauger commented Jan 4, 2024

In VDataTable, the options that are only numbers should not be sent to the i18n function t() as the keys do not exist causing warnings with i18n integrations.

Also, I'm not sure if there's another way, but having props like no-data-text expect a string that is a lcoale key and not a string seems counter intuitive? It's okay documented as such but the did work as expected in Vuetify 2 so perhaps it could be added to the upgrade guide. For example, doing :no-data-text="$t('somekey')" will look attempt to pass the retrun string of $t('somekey') to be passed through t again leading to a unknown key warning.

The itemsPerPageOptions lead to the following warnings in the console using vue-i18n:
[intlify] Not found '10' key in 'fr' locale messages.
[intlify] Fall back to translate '10' key with 'en' locale.
The warning is repeated for 25, 50, and 100.

There are a few ways to handle it, but it seems that the code should be changed to the following to work as initially intended using only a number in the default value.

  itemsPerPageOptions: {
    type: Array as PropType<readonly (number | { title: string | number, value: number })[]>,
    default: () => ([
-      { value: 10, title: '10' },
-      { value: 25, title: '25' },
-      { value: 50, title: '50' },
-      { value: 100, title: '100' },
+     10,
+     25,
+     50,
+     100,
-      { value: -1, title: '$vuetify.dataFooter.itemsPerPageAll' },
+   -1
    ]),
  },
  showCurrentPage: Boolean,
}, 'VDataTableFooter')

export const VDataTableFooter = genericComponent<{ prepend: never }>()({
  name: 'VDataTableFooter',

  props: makeVDataTableFooterProps(),

  setup (props, { slots }) {
    const { t } = useLocale()
    const { page, pageCount, startIndex, stopIndex, itemsLength, itemsPerPage, setItemsPerPage } = usePagination()

    const itemsPerPageOptions = computed(() => (
      props.itemsPerPageOptions.map(option => {
        if (typeof option === 'number') {
          return {
            value: option,
            title: option === -1
              ? t('$vuetify.dataFooter.itemsPerPageAll')
              : String(option),
          }
        }

        return {
          ...option,
         title: t(option.title),
        }
      })
    ))

Reference in repo:

https:/vuetifyjs/vuetify/blob/51f3f024a99e9a6c8ab5f8c29065f3e323e23b59/packages/vuetify/src/components/VDataTable/VDataTableFooter.tsx#L60C1-L98C7

@vincentauger vincentauger changed the title [Bug Report 3.4.9] VDataTable i18n page options [Bug Report][3.4.9] VDataTable i18n page options Jan 4, 2024
@websitevirtuoso
Copy link
Contributor

websitevirtuoso commented Jan 4, 2024

I may see the problem. but I need repoduction link. and after this I can do easy PR. so help from your side would be great

const itemsPerPageOptions = computed(() => (
      props.itemsPerPageOptions.map(option => {
        if (typeof option === 'number') {
          return {
            value: option,
            title: option === -1
              ? t('$vuetify.dataFooter.itemsPerPageAll')
              : String(option),
          }
        }

        return {
          ...option,
          title: t(option.title),
        }
      })
    ))

@vincentauger
Copy link
Contributor Author

Currently, the only main issue is that the default value of the options are as follow:

default: () => ([
      { value: 10, title: '10' },
      { value: 25, title: '25' },
      { value: 50, title: '50' },
      { value: 100, title: '100' },
      { value: -1, title: '$vuetify.dataFooter.itemsPerPageAll' },
    ]),

The current code sees this and tries to return title: t(option.title) which looks for a i18n key '10'. That key doesn't exists and as you mentioned, shouldn't either as you shouldn't translate numbers. If the default value is changed to:

default: () => ([10,25,50,100,-1]),

the computed property itemsPerPageOptions will work as intended and just return numbers as string without trying to pass them into the t function. It seems that when the computed property was refactored to accept numbers, the default value wasn't changed.

As I'm working through this V2 to V2 migration for my code, to avoid issues, I am now passing them to avoid the error as follows but wanted to flag the bug.

<v-data-table
                                   dense
                                    :headers="stationHeaders"
                                    :items="stations"
                                    :items-per-page="5"
                                    :items-per-page-options="[5, 10, 25, 50, -1]"
                                    sorty-by="id"
                                    item-key="id"
                                    :loading="stationsLoading"
                                >

@websitevirtuoso
Copy link
Contributor

Yes. but it didn't give me details. I need to reproduction link or repo. after that I can do PR and fix this problem

@vincentauger
Copy link
Contributor Author

Hello @websitevirtuoso - you can find a reproduction here based on documentation examples for setting up vue-i18n and a base Vuetify create cli app.

Reproduction Repo

Just run pnpm install && pnpm dev and launch dev server and you'll see the following warnings:

i18n-data-table

@websitevirtuoso
Copy link
Contributor

great. thanks

@vincentauger
Copy link
Contributor Author

Thank you for all the work you're doing here! 🚀

@websitevirtuoso
Copy link
Contributor

Created PR. Now need to wait reviewers

@websitevirtuoso websitevirtuoso linked a pull request Jan 5, 2024 that will close this issue
@johnleider johnleider added T: bug Functionality that does not work as intended/expected C: VDataTable VDatatable and removed S: triage labels Jan 8, 2024
@johnleider johnleider added this to the v3.4.x milestone Jan 8, 2024
@johnleider johnleider removed this from the v3.4.x milestone Jan 22, 2024
@xplloveyxl
Copy link

xplloveyxl commented Jan 30, 2024

i18n,build, {0} not parse
image

@xplloveyxl
Copy link

add options is good
VueI18nPlugin({
include: [path.resolve(__dirname, './src/locales/**/*.{json,yaml,yml}')],
jitCompilation: false,
compositionOnly: false,
runtimeOnly: false
}),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment