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

Merge custom translations into client app i18n #592

Merged
merged 6 commits into from
Jan 19, 2025

Conversation

kazoompa
Copy link
Member

No description provided.

@ymarcon
Copy link
Member

ymarcon commented Jan 18, 2025

  1. Empty list
  2. Add item
  3. Apply
system.ts:66 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'splice')
    at Proxy.updateTranslation (system.ts:66:40)
    at Proxy.wrappedAction (pinia.js?v=85b60f9c:1107:18)
    at store.<computed> (pinia.js?v=85b60f9c:785:44)
    at onApply (SystemCustomTranslations.vue:152:15)

Copy link
Member

@ymarcon ymarcon left a comment

Choose a reason for hiding this comment

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

Why is there an "Apply" action?

@kazoompa
Copy link
Member Author

Why is there an "Apply" action?

The intention is that user can apply several changes and apply those changes after - not to save change after every single little modifications.

@ymarcon
Copy link
Member

ymarcon commented Jan 18, 2025

Is it saved or not? If yes there is nothing to apply.

@kazoompa
Copy link
Member Author

Is it saved or not? If yes there is nothing to apply.

User makes some changes that are not saved but can do a batch of them and then apply the changes that will save into the DB.

@kazoompa
Copy link
Member Author

  1. Empty list
  2. Add item
  3. Apply
system.ts:66 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'splice')
    at Proxy.updateTranslation (system.ts:66:40)
    at Proxy.wrappedAction (pinia.js?v=85b60f9c:1107:18)
    at store.<computed> (pinia.js?v=85b60f9c:785:44)
    at onApply (SystemCustomTranslations.vue:152:15)

I cannot reproduce, maybe I am missing an initial state? Did you start from a clean install or yo had some translations? Also, was this running from 8081 or 9000?

@kazoompa
Copy link
Member Author

  1. Empty list
  2. Add item
  3. Apply
system.ts:66 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'splice')
    at Proxy.updateTranslation (system.ts:66:40)
    at Proxy.wrappedAction (pinia.js?v=85b60f9c:1107:18)
    at store.<computed> (pinia.js?v=85b60f9c:785:44)
    at onApply (SystemCustomTranslations.vue:152:15)

Was able to reproduce on a clean install... will fix.

@kazoompa kazoompa requested a review from ymarcon January 18, 2025 19:55
Copy link
Member

@ymarcon ymarcon left a comment

Choose a reason for hiding this comment

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

Selecting language tab does not change the table:
image

Then the dialog always refer to EN
image

Better UI/UX expected:
One dialog for all languages and one key

@kazoompa
Copy link
Member Author

kazoompa commented Jan 19, 2025

Selecting language tab does not change the table: image

Response
Did you add FR after? If so, the new language translations will inherit the EN texts (in the server).

Then the dialog always refer to EN image

Better UI/UX expected: One dialog for all languages and one key

Response
image

@kazoompa kazoompa requested a review from ymarcon January 19, 2025 13:29
Copy link
Member

@ymarcon ymarcon left a comment

Choose a reason for hiding this comment

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

Sorry to insist but UX is still not right:

  1. Single table, one column for the key and one column per language and remove these language tabs
  2. Update config on dialog's save or remove to; no need for this Apply thing

Please keep things simple.

@ymarcon
Copy link
Member

ymarcon commented Jan 19, 2025

image

@ymarcon ymarcon merged commit 6db37b8 into master Jan 19, 2025
2 checks passed
@ymarcon ymarcon deleted the feature/system-translations branch January 19, 2025 19:09
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.

2 participants