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

[TouchableHighlight] Preserve underlay style when restoring inactive props #129

Closed
wants to merge 1 commit into from
Closed

[TouchableHighlight] Preserve underlay style when restoring inactive props #129

wants to merge 1 commit into from

Conversation

ide
Copy link
Contributor

@ide ide commented Mar 7, 2015

If you give a TouchableHighlight component some styling (e.g. a background color) with the underlayStyle prop, the style is wiped away after touching the component. This diff restores the underlayStyle.

Test Plan: Create a TouchableHighlight that receives underlayStyle={{style: 'blue'}}. It initially has a blue background. Touch it and let go. See the blue background now comes back as expected.

this.refs[UNDERLAY_REF].setNativeProps(INACTIVE_UNDERLAY_PROPS);
var underlayProps = Object.assign({}, INACTIVE_UNDERLAY_PROPS);
underlayProps.style = this.state.underlayStyle;
this.refs[UNDERLAY_REF].setNativeProps(underlayProps);
Copy link
Contributor

Choose a reason for hiding this comment

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

this.refs[UNDERLAY_REF].setNativeProps({
  ...INACTIVE_UNDERLAY_PROPS,
  style: this.state.underlayStyle,
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's nicer, thanks @cpojer. If this diff gets merged in by hand can whoever merges it use the splat syntax? Otherwise I can update this PR if commits from the OSS repo can be directly pulled in now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to update the PR - it's trivial for us to apply internally and sync back out now.

On Mar 8, 2015, at 4:56 AM, James Ide notifications@github.com wrote:

In Libraries/Components/Touchable/TouchableHighlight.js:

@@ -156,7 +156,9 @@ var TouchableHighlight = React.createClass({
this._hideTimeout = null;
if (this.refs[UNDERLAY_REF]) {
this.refs[CHILD_REF].setNativeProps(INACTIVE_CHILD_PROPS);

  •  this.refs[UNDERLAY_REF].setNativeProps(INACTIVE_UNDERLAY_PROPS);
    
  •  var underlayProps = Object.assign({}, INACTIVE_UNDERLAY_PROPS);
    
  •  underlayProps.style = this.state.underlayStyle;
    
  •  this.refs[UNDERLAY_REF].setNativeProps(underlayProps);
    
    That's nicer, thanks @cpojer. If this diff gets merged in by hand can whoever merges it use the splat syntax? Otherwise I can update this PR if commits from the OSS repo can be directly pulled in now.


Reply to this email directly or view it on GitHub.

…props

If you give a TouchableHighlight component some styling (e.g. a background color) with the `underlayStyle` prop, the style is wiped away after touching the component. This diff restores the `underlayStyle`.

Test Plan: Create a TouchableHighlight that receives `underlayStyle={{style:
'blue'}}`. It initially has a blue background. Touch it and let go. See the blue background now comes back as expected.
@ide
Copy link
Contributor Author

ide commented Mar 10, 2015

Updated w/splat syntax and tested.

@jaygarcia
Copy link
Contributor

One thing that hit me was that this affects the ListView example. The delay is quite noticeable there too.

JG

301.785.6030 :: @moduscreate

:: sent from my mobile device ::

On Mar 10, 2015, at 18:35, James Ide notifications@github.com wrote:

Updated w/splat syntax and tested.


Reply to this email directly or view it on GitHub.

@ColinEberhardt
Copy link

Fixes #156

@vjeux
Copy link
Contributor

vjeux commented Mar 18, 2015

Pulled in
06a87be

Sorry, I don't know what happened but the commit title didn't appear in the list.

@vjeux vjeux closed this Mar 18, 2015
@ide
Copy link
Contributor Author

ide commented Mar 19, 2015

Thanks @vjeux!

@ide ide deleted the touchable-underlay branch March 19, 2015 08:06
vjeux pushed a commit to vjeux/react-native that referenced this pull request Apr 13, 2015
…props

Summary:
If you give a TouchableHighlight component some styling (e.g. a background color) with the `underlayStyle` prop, the style is wiped away after touching the component. This diff restores the `underlayStyle`.

Closes facebook#129
Github Author: James Ide <ide@jameside.com>

Test Plan:
 Create a TouchableHighlight that receives `underlayStyle={{style:
'blue'}}`. It initially has a blue background. Touch it and let go. See the blue background now comes back as expected.
vjeux pushed a commit to vjeux/react-native that referenced this pull request Apr 14, 2015
…props

Summary:
If you give a TouchableHighlight component some styling (e.g. a background color) with the `underlayStyle` prop, the style is wiped away after touching the component. This diff restores the `underlayStyle`.

Closes facebook#129
Github Author: James Ide <ide@jameside.com>

Test Plan:
 Create a TouchableHighlight that receives `underlayStyle={{style:
'blue'}}`. It initially has a blue background. Touch it and let go. See the blue background now comes back as expected.
vjeux pushed a commit to vjeux/react-native that referenced this pull request Apr 15, 2015
…props

Summary:
If you give a TouchableHighlight component some styling (e.g. a background color) with the `underlayStyle` prop, the style is wiped away after touching the component. This diff restores the `underlayStyle`.

Closes facebook#129
Github Author: James Ide <ide@jameside.com>

Test Plan:
 Create a TouchableHighlight that receives `underlayStyle={{style:
'blue'}}`. It initially has a blue background. Touch it and let go. See the blue background now comes back as expected.
ayushjainrksh referenced this pull request in MLH-Fellowship/react-native Jul 2, 2020
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.

6 participants