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

ComponentWillUnmout not called for Modal in AppleTV #15313

Closed
rada opened this issue Aug 1, 2017 · 10 comments
Closed

ComponentWillUnmout not called for Modal in AppleTV #15313

rada opened this issue Aug 1, 2017 · 10 comments
Labels
Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@rada
Copy link

rada commented Aug 1, 2017

Is this a bug report?

Yes

Have you read the Bugs section of the How to Contribute guide?

Yes

Environment

  1. react-native -v: 0.46.4
  2. node -v: 6.5.0
  3. npm -v: 3.10.3

Then, specify:

  • Target Platform: AppleTV

  • Development Operating System: macOS Sierra (10.12.5)

  • Build tools: Xcode (8.3.3)

Steps to Reproduce

Open Modal in Tab 1 by clicking on 'Open Modal' touchable.

(Write your steps here:)

  1. Focus touchable 'Open Modal' (in simulator press key Down)
  2. Press touchable 'Open Modal'
  3. Press 'Menu' button on Apple TV Remote (in simulator, press CMD + SHIFT + R to display TV Remote)

Expected Behavior

After the 'Menu' button is pressed on Apple TV Remote, Modal is closed and the method componentWillUnmout is called for component used in Modal.

Actual Behavior

Modal is closed but the componentWillUnmout method is not called if the modal is closed by pressing the 'Menu' button in the Apple TV Remote. If the modal is closed by pressing the touchable 'Close Modal', the componentWillUnmout method is called as expected.

Reproducible Demo

https://gitlab.com/radamich/tvTabBar/tree/tvosModal

index

/**
 * Sample React Native App
 * https://github.com/facebook/react-native
 * @flow
 */

import React, { Component } from 'react';
import {
  AppRegistry,
  Text,
  View,
  TabBarIOS,
  ListView,
  Modal,
  TouchableHighlight,
} from 'react-native';

import List from './List';

export default class tvTabBar extends Component {

	_tabItems: Array<{ key: string }> = [
		{ key: 'TAB 1' },
		{ key: 'TAB 2' },
		{ key: 'TAB 3' },
		{ key: 'TAB 4' },
	];

	constructor(props) {
		super(props);
		this.state = {
			activeTab: 'TAB 1',
			showModal: false,
		};
	}

	_getTabItem = (key: string) => {
		switch (key) {
			case 'TAB 1':
				return (
					<View style={{ flex: 1, top: 140, borderColor: 'red', borderWidth: 10, padding: 200 }}>
						<Modal 
							visible={this.state.showModal}
							transparent={true}
						>
							<List closeModal={this.closeModal} />

						</Modal>
						<TouchableHighlight onPress={() => {
								console.log('MODAL SHOW', this.state.showModal);
								this.setState({ showModal: true });
							}}
							underlayColor={'orange'}
						>
							<Text style={{ fontSize: 30 }}>Open Modal</Text>
						</TouchableHighlight>
					</View>
				);
			case 'TAB 2':
				return (
					<View style={{ flex: 1, top: 140, borderColor: 'green', borderWidth: 20 }}>
						<Text style={{ fontSize: 30 }}>TAB 2</Text>
					</View>
				);
			case 'TAB 3':
				return (
					<View style={{ flex: 1, top: 140, borderColor: 'darkblue', borderWidth: 30 }}>
						<Text style={{ fontSize: 30 }}>TAB 3</Text>
					</View>
				);
			case 'TAB 4':
				return (
					<View style={{ flex: 1, top: 140, borderColor: 'deeppink', borderWidth: 40 }}>
						<Text style={{ fontSize: 30 }}>TAB 4</Text>
					</View>
				);
			default:
				return null;
		}
	}

	closeModal = () => {
		this.setState({ showModal: false });
	}

	_setTab(key) {
		return () => {
			console.log('Selecting tab: ', key);
			if (this.state.activeTab !== key) {
				this.setState({ activeTab: key });
			}
		};
	}

	render() {
		return (
			<TabBarIOS>
				{
					this._tabItems.map((tabItem) => {
						return (
							<TabBarIOS.Item
								key={tabItem.key}
								onPress={this._setTab(tabItem.key)}
								title={tabItem.key}
								selected={tabItem.key === this.state.activeTab}
							>
								{ this._getTabItem(tabItem.key) }
							</TabBarIOS.Item>
						);
					})
				}
			</TabBarIOS>
		);
	}
}

AppRegistry.registerComponent('tvTabBar', () => { return tvTabBar; });

List.js

/**
 * Sample React Native App
 * https://github.com/facebook/react-native
 * @flow
 */

import React, { Component } from 'react';
import {
  Text,
  View,
  ListView,
  FlatList,
  TouchableHighlight,
} from 'react-native';

export default class List extends Component {

	
	constructor(props) {
		super(props);

		this.ds = new ListView.DataSource({ rowHasChanged: (r1: Object, r2: Object) => { return r1 !== r2; } });

		this.state = {
			dataSource: this.ds.cloneWithRows([{ value: 1 }]),
			data: [{ value: 1 }, { value: 2 }],
		}
	};

	_renderRow(rowData) {
		console.log('RenderRow: ', rowData.value);
		return (
			<View>
				<Text>{rowData.value}</Text>
			</View>
		)
	}
	
	_renderFlatListItem = ({ item }) => {
		return this._renderRow(item);
	}
	
	componentDidMount() {
		setTimeout(() => {
			const data = Array.from({ length: 2000 }).map((_, index) => {
				return { value: index };
			})
			this.setState({
				dataSource: this.ds.cloneWithRows(data),
				// data: data,
			})
		}, 3000)
	}

	componentWillUnmount() {
		console.log('List.js: componentWillUnmount');
	}
	
	render() {
		console.log('Render List');
		return (
			<View style={{ backgroundColor: 'blue', padding: 500 }}>
				<FlatList
					data={this.state.data}
					renderItem={this._renderFlatListItem}
					keyExtractor={(item) => { return item.value }}
				/>
				<TouchableHighlight onPress={() => {
						this.props.closeModal();
					}}
					underlayColor={'orange'}
				>
					<Text style={{ fontSize: 30 }}>Close Modal</Text>
				</TouchableHighlight>
			</View>
		);
	}
}
@rada
Copy link
Author

rada commented Aug 1, 2017

@dlowder-salesforce

@douglowder
Copy link
Contributor

Thanks for the bug report @rada , I'll have a look

@douglowder
Copy link
Contributor

Hmmm this does not look like something that will be easy to fix. The Modal component isn't really a modal in the usual iOS sense -- it just renders null if visible is set to false, and renders normally otherwise. The problem is that the tvOS menu button bypasses the JS and just dismisses the view, getting the app into a bad state where JS thinks the Modal component is still there, but there is no native view for it any more. I think this component needs some work maybe....

@douglowder
Copy link
Contributor

Found a solution -- will be submitting a pull request shortly.

@douglowder
Copy link
Contributor

@rada you can test out my fix by using https://github.com/dlowder-salesforce/react-native/tree/tvos-combined-fixes (includes previous fixes to TabBarIOS).

You will need to add one line of code to your test case: in index.ios.js, the Modal component should look as shown below:

						<Modal 
							visible={this.state.showModal}
                                                        onMenuPress={this.closeModal}    // <------
							transparent={true}
						>
							<List closeModal={this.closeModal} />

						</Modal>

@rada
Copy link
Author

rada commented Aug 2, 2017

Thanks @dlowder-salesforce, it's working now with your fix. Thanks.

@douglowder
Copy link
Contributor

@rada in further work on this issue, I find that I need to change the onMenuPress property to onRequestClose instead, for consistency with Android (where the back button does the same thing). My repo has been updated with this change.

facebook-github-bot pushed a commit that referenced this issue Aug 17, 2017
…5313)

Summary:
**Motivation**

On Apple TV, pressing the menu button destroys the native view that backs the `Modal` component, causing an app using this component to get into a broken state.  This fix implements `onRequestClose` for tvOS to have the same behavior as it does for the Android back button.

**Test plan**

Manually tested this with the `ModalExample` in the `RNTester` app.  See also the test code in issue #15313.
Closes #15341

Differential Revision: D5651035

Pulled By: shergin

fbshipit-source-id: 54bf66887bbe85940567e63e90b437ac4a8daf9a
@douglowder
Copy link
Contributor

The fix for this is now merged, so we can close this issue.

@stale
Copy link

stale bot commented Oct 18, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Oct 18, 2017
@stale stale bot closed this as completed Oct 25, 2017
@psegalen
Copy link

psegalen commented Mar 8, 2018

Hi all, sorry for bringing bad news but it seems that @dlowder-salesforce's onRequestClose gets called on tvOS only for "Debug" scheme, in "Release" it just doesn't fire!

Repo to reproduce: https://github.com/psegalen/rn-tvos-modal-bug
Steps:

  • Get the code, run it in Apple TV simulator with XCode (btw, to get an app running in Release scheme for tvOS you have to modify a react-native init config twice, one to add libReact.a to the test project, another to add missing "Other Linker Flag" -lc++) (Posted a new issue ("react-native init" creates tvOS projects that won't run for Release scheme #18288))
  • If you press the touchable, the modal shows, if you press the remote's menu button, it hides, it's ok but onRequestClose isn't called (you can see that because the state hasn't changed, isModalOpen remains true)
  • If you do the same test after switching the "Run" scheme to "Debug", you'll see that the behaviour is correct since the state is correctly changed (isModalOpen become false after closing the modal)

Can someone check why this is happening please?
My app is behaving as expected when I'm in debug mode but not on an actual device installed via TestFlight. :(

Posted a new issue (#18289)

@facebook facebook locked and limited conversation to collaborators May 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

No branches or pull requests

3 participants