Allow passing a ref prop to the Text component #2909

Aniganesh · 2025-03-06T09:45:01Z

Description

Issues: fixes Allow passing ref to Text component in Ignite boilerplate #2758

Screenshots

Demo page:

Checklist

I have manually tested this, including by generating a new app locally (see docs).

coolsoftwaretyler

Hey @Aniganesh - this looks great. Thanks for writing it up against the latest Ignite. I see you had submitted #2765 earlier. We appreciate your time!

I have a couple questions, thoughts, suggestions. I don't see anything that I consider a blocker, but I'll see if anyone else from IR wants to take a look before merge.

coolsoftwaretyler · 2025-03-07T01:10:31Z

boilerplate/app/components/Text.tsx

 */
-export function Text(props: TextProps) {
+export const Text = forwardRef(function Text(
+  props: PropsWithChildren<TextProps>,


Suggested change

props: PropsWithChildren<TextProps>,

props: TextProps,

suggestion: I don't think we need PropsWithChildren here since we already have it in the TextProps interface

coolsoftwaretyler · 2025-03-07T01:11:03Z

boilerplate/app/components/Text.tsx

-export function Text(props: TextProps) {
+export const Text = forwardRef(function Text(
+  props: PropsWithChildren<TextProps>,
+  ref: ForwardedRef<RNText | null>,


Suggested change

ref: ForwardedRef<RNText | null>,

ref: ForwardedRef<RNText>,

suggestion: I don't think we want null here. RNText should suffice.

coolsoftwaretyler · 2025-03-07T01:13:07Z

boilerplate/app/screens/DemoShowroomScreen/demos/DemoText.tsx

+import { useAppTheme } from "@/utils/useAppTheme"
+
+// Define the TextRefDemo component outside of other function components
+const TextRefDemo = () => {


praise: nice! Thanks for the demo.

coolsoftwaretyler · 2025-03-07T01:14:39Z

boilerplate/app/screens/DemoShowroomScreen/demos/DemoText.tsx

+import { useAppTheme } from "@/utils/useAppTheme"
+
+// Define the TextRefDemo component outside of other function components
+const TextRefDemo = () => {


suggestion: maybe we should extract TextRefDemo to a separate file? I feel very neutral on this, but the other demo files seem to just be the demo objects. I'm not sure if there's a good example of something like this, though.

coolsoftwaretyler · 2025-03-07T01:16:38Z

boilerplate/app/i18n/demo-fr.ts

  demoIcon: {
    description:
-      "Un composant pour faire le rendu d’une icône enregistrée. Il est enveloppé dans un <TouchableOpacity /> si `onPress` est fourni, sinon dans une <View />.",
+      "Un composant pour faire le rendu d'une icône enregistrée. Il est enveloppé dans un <TouchableOpacity /> si `onPress` est fourni, sinon dans une <View />.",


Suggested change

"Un composant pour faire le rendu d'une icône enregistrée. Il est enveloppé dans un <TouchableOpacity /> si `onPress` est fourni, sinon dans une <View />.",

"Un composant pour faire le rendu d’une icône enregistrée. Il est enveloppé dans un <TouchableOpacity /> si `onPress` est fourni, sinon dans une <View />.",

suggestion: can we revert these changes? I'm not sure how much it matters, but it looks like maybe you had a format-on-save change a bunch of unrelated characters in the french localization file.

coolsoftwaretyler · 2025-03-07T01:17:51Z

boilerplate/app/i18n/demo-ar.ts

        text3:
          "Eiusmod occaecat laboris eu ex veniam ipsum adipisicing consectetur. Magna ullamco adipisicing tempor adipisicing.",
      },
+      refs: {


question: @frankcalise - do we have a process for reviewing localization files across languages? I can run this through google translate but I'm curious if we've got a better mechanism for that across each locale.

Yeah it isn't as simple as just pushing strings through Google Translate, we've reached out to various parts of the community for official translations.

I'm going to say we don't need to have a use case for this demo, just accept the forwardRef on the component and that should tidy up this PR a bunch.

coolsoftwaretyler · 2025-03-07T01:18:04Z

boilerplate/app/i18n/demo-en.ts

        text3:
          "Eiusmod occaecat laboris eu ex veniam ipsum adipisicing consectetur. Magna ullamco adipisicing tempor adipisicing.",
      },
+      refs: {


praise: thank you for doing the localization!

coolsoftwaretyler · 2025-03-07T01:24:49Z

boilerplate/app/screens/DemoShowroomScreen/demos/DemoText.tsx

+      setHighlighted(true)
+
+      // Reset highlight after a delay
+      setTimeout(() => {


suggestion: rather than resetting the highlight, can we make the button toggle the focus? This is a personal preference - sometimes I look away and miss a time-based demo like this. Not a blocker.

frankcalise

Thanks for updating your contribution!

If you wouldn't mind, can we revert the i18n and DemoText.tsx changes. It doesn't feel necessary to demo a ref since it's not really something Ignite is offering over how React works.

It'll also help keep the maintenance down for translating more strings and we'd also have to even verify that the ones here are valid.

frankcalise · 2025-03-07T14:30:36Z

boilerplate/app/i18n/demo-ar.ts

        text3:
          "Eiusmod occaecat laboris eu ex veniam ipsum adipisicing consectetur. Magna ullamco adipisicing tempor adipisicing.",
      },
+      refs: {


Yeah it isn't as simple as just pushing strings through Google Translate, we've reached out to various parts of the community for official translations.

I'm going to say we don't need to have a use case for this demo, just accept the forwardRef on the component and that should tidy up this PR a bunch.

Aniganesh · 2025-03-07T14:34:23Z

Will update

…

On Fri, 7 Mar, 2025, 8:02 pm Frank Calise, ***@***.***> wrote: ***@***.**** requested changes on this pull request. Thanks for updating your contribution! If you wouldn't mind, can we revert the i18n and DemoText.tsx changes. It doesn't feel necessary to demo a ref since it's not really something Ignite is offering over how React works. It'll also help keep the maintenance down for translating more strings and we'd also have to even verify that the ones here are valid. ------------------------------ In boilerplate/app/i18n/demo-ar.ts <#2909 (comment)>: > @@ -393,6 +393,13 @@ export const demoAr: DemoTranslations = { text3: "Eiusmod occaecat laboris eu ex veniam ipsum adipisicing consectetur. Magna ullamco adipisicing tempor adipisicing.", }, + refs: { Yeah it isn't as simple as just pushing strings through Google Translate, we've reached out to various parts of the community for official translations. I'm going to say we don't need to have a use case for this demo, just accept the forwardRef on the component and that should tidy up this PR a bunch. — Reply to this email directly, view it on GitHub <#2909 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALMA3WVWO77NDETPEFKDZZD2TGUXNAVCNFSM6AAAAABYOG6TYSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMNRXGQYTONJWGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

…i18n

frankcalise · 2025-03-08T22:53:33Z

boilerplate/app/i18n/demo-fr.ts

I think you made some unintended changes here to this file

frankcalise

Thank you!

@Aniganesh

## [10.1.9](v10.1.8...v10.1.9) (2025-03-12) ### Bug Fixes * **boilerplate:** Allow passing a ref prop to the Text component ([#2909](#2909) by @Aniganesh) ([a1a220b](a1a220b))

infinitered-circleci · 2025-03-12T15:33:37Z

🎉 This PR is included in version 10.1.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

aniruddha-mithya added 2 commits March 6, 2025 15:09

Update post ignite X release. Add useful demo

4a61634

fix formatting issues

809056e

Aniganesh changed the title ~~Allow passing a ref to the Text prop~~ Allow passing a ref prop to the Text component Mar 6, 2025

Aniganesh marked this pull request as ready for review March 6, 2025 09:52

coolsoftwaretyler self-requested a review March 7, 2025 01:05

coolsoftwaretyler reviewed Mar 7, 2025

View reviewed changes

frankcalise requested changes Mar 7, 2025

View reviewed changes

aniruddha-mithya added 4 commits March 8, 2025 11:08

Remove unnecessary demo, translations, undo unintended changes to fr …

cc2841f

…i18n

fix formatting

40b6ced

fix failing test

f1881f4

remove unused imports

721aa18

frankcalise requested changes Mar 8, 2025

View reviewed changes

restore translations to earlier state

5a44969

frankcalise approved these changes Mar 12, 2025

View reviewed changes

frankcalise merged commit a1a220b into infinitered:master Mar 12, 2025
1 check passed

infinitered-circleci added the released label Mar 12, 2025

	"Un composant pour faire le rendu d'une icône enregistrée. Il est enveloppé dans un <TouchableOpacity /> si `onPress` est fourni, sinon dans une <View />.",
	"Un composant pour faire le rendu d’une icône enregistrée. Il est enveloppé dans un <TouchableOpacity /> si `onPress` est fourni, sinon dans une <View />.",

Allow passing a ref prop to the Text component #2909

Allow passing a ref prop to the Text component #2909

Uh oh!

Conversation

Aniganesh commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Screenshots

Checklist

Uh oh!

coolsoftwaretyler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankcalise left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aniganesh commented Mar 7, 2025 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankcalise left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

infinitered-circleci commented Mar 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Aniganesh commented Mar 6, 2025 •

edited

Loading