Skip to content

[TextInput] Fixes #1509 -- Add note about side-specific properties on TextInput #2240

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[TextInput] Fixes #1509 -- Add note about side-specific properties on TextInput #2240

wants to merge 1 commit into from

Conversation

pletcher
Copy link
Contributor

@pletcher pletcher commented Aug 6, 2015

Add note about side-specific properties not being applied to TextInputs if multiline=false.

Fix formatting of docs describing other multiline={true/false} quirks.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Aug 6, 2015
@pletcher pletcher mentioned this pull request Aug 11, 2015
Closed
@brentvatne
Copy link
Collaborator

Can you rebase this @pletcher? Wow sorry this got ignored

@pletcher
Copy link
Contributor Author

pletcher commented Jan 3, 2016

Yes, for sure! And no worries!

On Sun, Jan 3, 2016 at 12:37 AM, Brent Vatne [email protected]
wrote:

Can you rebase this @pletcher https://github.com/pletcher? Wow sorry
this got ignored

—
Reply to this email directly or view it on GitHub
#2240 (comment)
.

@facebook-github-bot
Copy link
Contributor

@pletcher updated the pull request.

@pletcher
Copy link
Contributor Author

Whoa, weird that Travis errored out. I'll rebase again.

@facebook-github-bot
Copy link
Contributor

@pletcher updated the pull request.

@mkonicek
Copy link
Contributor

mkonicek commented Feb 9, 2016

Are the comments "// not supported in Open Source yet" still relevant? Sorry this got missed, we have a bot that mentions people who are likely good reviewers now.

@ghost
Copy link

ghost commented Mar 3, 2016

@sahrens would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

@mkonicek
Copy link
Contributor

mkonicek commented Mar 3, 2016

@pletcher Are the comments "// not supported in Open Source yet" still relevant?

I don't understand this format:

var onlyMultiline = {
    onSelectionChange: true, // not supported in Open Source yet
    onTextInput: true, // not supported in Open Source yet
    children: true,
};

Is true a valid argument for onSelectionChange?

What does the comment explain? Is it saying that the onSelectionChange callback only works if multiline={true}? Does the JS object onlyMultiline have some other meaning?

@mkonicek
Copy link
Contributor

mkonicek commented Mar 3, 2016

Also, since this was sent on Aug 6, are the fact in the comment still valid? Sorry this PR got lost among the GitHub notifications. We've implemented a bot that will ping people on PRs, didn't have it back when this PR was created.

@pletcher
Copy link
Contributor Author

pletcher commented Mar 3, 2016

Hm, good questions. That format was left in from a previous commit. I'll
take a look at all of this tonight and update accordingly -- sorry for the
delay!

On Thursday, March 3, 2016, Martin Konicek [email protected] wrote:

Also, since this was sent on Aug 6, are the fact in the comment still
valid? Sorry this slipped through the cracks!

—
Reply to this email directly or view it on GitHub
#2240 (comment)
.

@mkonicek
Copy link
Contributor

mkonicek commented Mar 4, 2016

No worries @pletcher. We've just implemented the bot that pings old PRs so they don't get forgotten about.

@pletcher
Copy link
Contributor Author

pletcher commented Mar 8, 2016

Looks like that onlyMultiline object was removed in 3704257 -- it was included when I first added my note, but I must have included it in a rebase down the line.

@pletcher
Fixes #1509
d5f382a 
Add note about side specific properties not being applied to TextInputs
if multiline=false.

Fix formatting of docs describing other multiline={true/false} quirks.

Remove stale notes, update comment about border styles
@facebook-github-bot
Copy link
Contributor

@pletcher updated the pull request.

@ghost
Copy link

ghost commented Mar 9, 2016

@AaaChiuuu would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

@mkonicek
Copy link
Contributor

cc @andreicoman11. If this PR is not relevant anymore please close.

@andreicoman11
Copy link
Contributor

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to Phabricator to review.

ghost pushed a commit that referenced this pull request Mar 10, 2016
@pletcher
Fixes #1509 -- Add note about side-specific properties on TextInput
f5a1600 
Summary:Add note about side-specific properties not being applied to `TextInput`s if `multiline=false`.

Fix formatting of docs describing other `multiline={true/false}` quirks.
Closes #2240

Differential Revision: D3037154

Pulled By: andreicoman11

fb-gh-sync-id: d64db906c2ab0054f2357b8e218725414d9868d6
shipit-source-id: d64db906c2ab0054f2357b8e218725414d9868d6
gabrieldonadel pushed a commit to gabrieldonadel/react-native that referenced this pull request Feb 22, 2025
@tido64
chore(init): use a stricter tsconfig.json (facebook#2240)
fec7910
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@andreicoman11 andreicoman11

Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pletcher @brentvatne @facebook-github-bot @mkonicek @andreicoman11