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

Add the option to style the container of the text View. #3

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

Conversation

JackThomson2
Copy link
Contributor

The changes shouldn't break existing code.

README.md Outdated
@@ -41,4 +45,5 @@ import AutoTypingText from 'react-native-auto-typing-text';
|**`charMovingTime`**|`number`|time to type each character|
|**`delay`**|`number`|Delay time before typing|
|**`style`**|`string`|Style for text|
|**`containerStyle`**|`string`|Style for text container|

Choose a reason for hiding this comment

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

containerStyle must be object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that may more sense but tried to match the previous, i'll make the change now

Choose a reason for hiding this comment

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

Yep, I thought u copy pasted as it was same 😂

Choose a reason for hiding this comment

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

While you are at it, change this too. It should be a number not string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops sorry didn't see that I'll add another commit

README.md Outdated
@@ -41,4 +45,5 @@ import AutoTypingText from 'react-native-auto-typing-text';
|**`charMovingTime`**|`number`|time to type each character|
|**`delay`**|`number`|Delay time before typing|
|**`style`**|`string`|Style for text|

Choose a reason for hiding this comment

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

style must be object

Copy link

@deadcoder0904 deadcoder0904 left a comment

Choose a reason for hiding this comment

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

Now good to go 🎉 🎉

@JackThomson2
Copy link
Contributor Author

@deadcoder0904 just wondering if we can drop the HiddenText.js completely not sure of it's use?

@deadcoder0904
Copy link

Naah I guess HiddenText actually calculates the size required to display it & divides it in chunks. I guess @phuongla can answer this better

@ngothanhtai
Copy link
Owner

@deadcoder0904 yes, that's right. It's the purpose of HiddenText.

@JackThomson2
Copy link
Contributor Author

Great 👍 @ngothanhtai this good to be pulled in?

<View style={[styles.flex1]}>
<Text { ...this.props }>
<View style={this.props.containerStyle}>
<Text style={this.props.style}>
Copy link
Owner

Choose a reason for hiding this comment

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

@JackThomson2 some props (selectable, accessible, etc.), users need to pass to the Text component so I prefer to keep { ...this.props }.

Copy link
Owner

Choose a reason for hiding this comment

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

@JackThomson2 just give you a comment. please kindly review :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sure no problem I'll make the change when I get a chance 👍

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