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

Use i18n Keys (2) #4464

Merged
merged 8 commits into from
Nov 7, 2024
Merged

Use i18n Keys (2) #4464

merged 8 commits into from
Nov 7, 2024

Conversation

danicruz0415
Copy link
Contributor

@danicruz0415 danicruz0415 commented Oct 18, 2024

End-user friendly description of the problem this fixes or functionality that this introduces
Work in progress. Updates interface components to use i18next keys so that the user can view them in different languages. See also this PR

Link of any specific issues this addresses

#4280

This PR is meant to cover all the folders that are missing translations. The files outside the folders are being handled here

🟦 means does not need localization
🟩 means its already localized
🟥 means needs localization

  • 📁 buttons 🟦
  • 📁 chat 🟩
  • 📁 context-menu 🟦
  • 📁 file-explorer 🟩
  • 📁 form 🟥
    • ⚛️ custom-input.tsx
    • ⚛️ FormFieldset.tsx
    • ⚛️ settings-form.tsx
  • 📁 markdown 🟦
  • 📁 modals 🟥
    • 📁 base-modal
    • 📁 confirmation-modals
    • 📁 feedback
    • 📁 security
    • ⚛️ settingsAccountSettingsModal.tsx
    • ⚛️ connect-to-github-modal.tsx
    • ⚛️ ConnectToGitHubByTokenModal.tsx
    • ⚛️ LoadingProject.tsx
    • ⚛️ modal-backdrop.tsx
    • ⚛️ ModalBody.tsx
  • 📁 project-menu 🟥
    • ⚛️ project-menu-details-placeholder.tsx
    • ⚛️ project-menu-details.tsx
    • ⚛️ project.menu-card-context-menu.tsx
    • ⚛️ ProjectMenuCard.tsx
  • 📁 terminal 🟦

@danicruz0415 danicruz0415 mentioned this pull request Oct 18, 2024
Copy link
Member

@amanape amanape left a comment

Choose a reason for hiding this comment

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

You're on the right path! It would be helpful if you could also update your description to include the files and directories you plan on covering, e.g.,

  • modals
  • forms

And so on...

This will help sync with @Vaishakh-SM and reduce chances of conflicts

frontend/src/components/modals/LoadingProject.tsx Outdated Show resolved Hide resolved
@danicruz0415
Copy link
Contributor Author

Hi @amanape sorry for the time away, I was a bit bussy. I updated the description with the files I'm taking and its current status. I wanted to ask you something before advancing more, I realized there are some resources that have a standard where the name is the resource plus the context in which they are used. i.e:

  • CONFIGURATION$SECURITY_SELECT_LABEL
  • CONFIGURATION$MODAL_CLOSE_BUTTON_LABEL
  • CONFIGURATION$SECURITY_SELECT_PLACEHOLDER
    (note the "label" and "placeholder" at the end)

I saw some other resources that don't hold this standard and just have a brief piece of the text. i.e:

  • CONFIGURATION$AGENT_LOADING
  • CONFIGURATION$AGENT_RUNNING

I know this does not affect functionality but I am an advocate of standards 😅 so I like to keep things consistent
I know its hard across such large projects (I realized that the file naming is also sometimes with-dashes and sometimes with camelCase)

So I wanted to know if you think this is a standard worth keeping or I better use just the name being a part of the text. I think those sufixes can make the name longer, and in context, the resource should not care where its used, but what it says (as long as the context does not change the meaning). Keeping those sufixes can reduce reusability: for example if a label and a message both say "success", with the suffixes you might need to create two resources "SUCCESS_LABEL" and "SUCCESS_MESSAGE" even though both could just be one single resource "SUCCESS". But if you know a reason to keep them let me know to see if its needed to update the translations I've done so far. Thank you. @Vaishakh-SM I'd like to hear also your opinion 🙈 .

@amanape
Copy link
Member

amanape commented Nov 4, 2024

Yes sure, enforcing standards is always a good practice!

@danicruz0415 danicruz0415 marked this pull request as ready for review November 6, 2024 01:59
@danicruz0415 danicruz0415 changed the title [WIP] Use i18n Keys (2) Use i18n Keys (2) Nov 6, 2024
@danicruz0415
Copy link
Contributor Author

Hi @amanape
This PR is ready for review. I added the resources for the files in the description. I also added the translations to spanish.
I am not uploading the OpenHands/frontend/src/i18n/declaration.ts file as its an autogenerated file, but let me know if you want me to upload it (Currently without it some resources look like they don't exist).

Github also is telling me that there are conflics with 3 files, let me know if you can resolve them or I have to update locally to resolve them.

I stay tuned for any feedback, thank you 😄
@Vaishakh-SM to keep you posted

@amanape
Copy link
Member

amanape commented Nov 6, 2024

Hey @danicruz0415, I am unable to resolve the conflicts because the changes are in your forked repository instead of a branch of this one. You're going to have to resolve them yourself but note that frontend/src/components/modals/feedback/FeedbackModal.tsx has been deleted in the main, so it is safe to remove.

@danicruz0415
Copy link
Contributor Author

Hi @amanape I think I just resolved the conflicts, first time with that scary message "These conflicts are too complex to resolve in the web editor" 😅.
Can you take a look? 🥲

Copy link
Member

@amanape amanape left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks a lot

@amanape amanape enabled auto-merge (squash) November 7, 2024 08:11
@amanape amanape merged commit bb362cd into All-Hands-AI:main Nov 7, 2024
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