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

Allow passing ref to Text component in Ignite boilerplate #2758 #2765

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

Conversation

Aniganesh
Copy link

@Aniganesh Aniganesh commented Sep 6, 2024

Please verify the following:

  • yarn test jest tests pass with new tests, if relevant
  • yarn lint eslint checks pass with new code, if relevant
  • yarn format:check prettier checks pass with new code, if relevant
  • README.md (or relevant documentation) has been updated with your changes

Describe your PR

This PR allows passing ref to the Text component in boilerplate.
Have added the feature and a demo in Demo Community Screen.
@frankcalise Let me know if I need to add something or if things need to be moved around.

@Aniganesh
Copy link
Author

This fixes #2758

@lindboe
Copy link
Contributor

lindboe commented Sep 10, 2024

Hi @Aniganesh!

For understanding your PR and improving documentation, could you let us know your use case for forwarding the Text ref?

@Aniganesh
Copy link
Author

Aniganesh commented Sep 30, 2024

Hey! Extremely sorry for not responding for this long. The use case for this was that I had to position a bunch of <Text /> components and also adjust their positioning based on their heights. Initially I used refs and then called measure() on the refs. That's when I got the idea for the PR. Later, I realized my purpose is better served with the onLayout callback and went with that instead. Thought I'd still add the PR in case anyone has a similar purpose and wants to use refs.

Once again, apologies for not responding for so long.

Copy link
Contributor

@lindboe lindboe left a comment

Choose a reason for hiding this comment

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

Hey @Aniganesh , thanks again for doing this! Sorry it took a minute to get back to you -- we just released a new major version of Ignite and have been focusing on that!

Since that happened, this PR does have some version conflicts. If you want to update it go ahead, if not, no worries, just let us know!

return (
<Screen preset="scroll" contentContainerStyle={$container} safeAreaEdges={["top"]}>
<Text preset="heading" tx="demoCommunityScreen.title" style={$title} />
<Text preset="heading" tx="demoCommunityScreen.title" style={$title} ref={textRef} />
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if we're not using the ref for anything, we don't need to add it to the demo here.

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