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

feat: add dynamic params to text #384

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

elalami-m
Copy link

What does this do?

This PR extends the Text component to support translation parameters by modifying the tx prop to accept an object with a translation key and parameters. This allows developers to use dynamic, interpolated values within translations (e.g., inserting names or numbers into strings).

Why did you do this?

This change was made to improve the flexibility of the Text component, particularly for internationalization. In many cases, translations require dynamic content (e.g., greetings with a user’s name), and this update allows developers to pass those parameters directly through the component. It simplifies the code needed for multi-language support and enhances readability.

Who/what does this impact?

This change impacts developers using the Text component for localized content, as it enables dynamic translations without requiring custom workarounds. This is especially relevant for teams working on multilingual applications that need flexibility in handling text. It should not introduce any breaking changes to existing functionality, as it maintains backward compatibility with the previous tx prop type.

How did you test this?

  • Verified that the Text component correctly renders static translations as before.
  • Tested the component with translations that include dynamic parameters, ensuring they render as expected.
  • Checked the component in RTL and LTR layouts to confirm no side effects with the added parameter support.

Copy link

netlify bot commented Nov 8, 2024

Deploy Preview for obytes-starter canceled.

Name Link
🔨 Latest commit e1323f1
🔍 Latest deploy log https://app.netlify.com/sites/obytes-starter/deploys/672e1312dcd748000848badf

@yjose yjose self-requested a review November 8, 2024 11:53
Copy link
Member

@yjose yjose left a comment

Choose a reason for hiding this comment

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

Sounds good, but can we ensure we add the correct type for the option to avoid using any?

Also the name params is a bit misleading, i would prefer using options instead

@elalami-m
Copy link
Author

Thank you for the feedback, @yjose! 🙏

I’ve updated the code to replace any with a more specific type ({ [key: string]: string }) for the options. I also renamed params to options as you suggested to improve clarity.

Let me know if there’s anything else I can adjust.

@yjose
Copy link
Member

yjose commented Nov 8, 2024

@elalami-m Good! I would love to make the option more accurate by adding the correct type. I think it should be TranslateOptions. You can check this file.

@yjose
Copy link
Member

yjose commented Nov 9, 2024

@elalami-m I would also appreciate it if you could add an example for translations as part of the style screen, similar to what we have for typography and buttons. By providing the different values available for text with translation and options, we can enhance clarity.

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

Successfully merging this pull request may close these issues.

2 participants