Skip to content

Navigation API extracted to the extension #145

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

Merged
merged 12 commits into from
Aug 24, 2017

Conversation

dryganets
Copy link
Contributor

@dryganets dryganets commented Jun 23, 2017

Replace Navigator API with extension.

We moved to react-native-deprecated-custom-components for StandardNavigationDelegate and to reactxp-experimental-navigation for ExperimentalNavigationDelegate.

There is a breaking change in the standard navigator. Styles should be passed as objects to the renderScene method.

facebook/react-native@febf3d0

@msftclas
Copy link

@dryganets,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@dryganets dryganets force-pushed the sergeyd/remove-navigator-api branch from fbba079 to f4b37a8 Compare August 23, 2017 20:28
StandardDelegate uses react-native-deprecated-components Navigator.
@dryganets dryganets force-pushed the sergeyd/remove-navigator-api branch from 2aaa063 to c1a762a Compare August 24, 2017 01:11
@dryganets dryganets force-pushed the sergeyd/remove-navigator-api branch from 9835b4e to 13032ea Compare August 24, 2017 07:02
@dryganets dryganets mentioned this pull request Aug 24, 2017
…s out that deprecated version of Navigator is not compatible with react-native 0.42.
…h corrected imports. Some samples doesn't work with 1.0.4
"typescript": "2.4.1",
"tslint": "^5.0.0"
},
"author": "Sergei Dryganets",
Copy link
Collaborator

Choose a reason for hiding this comment

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

"ReactXP Team [email protected]"

@@ -10,17 +10,24 @@
* pop, which update the state and cause transitions.
*/

import _ = require('./utils/lodashMini');
import _ = require('lodash');
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still think we want lodashmini for best bundled size

"experimentalDecorators": true,
"noImplicitAny": true,
"outDir": "dist/",
"lib": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

if it's easy, please try enabling strictNullChecks here

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 fixed it for react-native. For the web, it is way more tricky, and I don't have enough context for it now.

],
"dependencies": {
"@types/lodash": "4.14.62",
"@types/node": "6.0.65",
Copy link
Collaborator

Choose a reason for hiding this comment

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

use semver '^' throughout so consumers don't get stuck on a particular version

Copy link

@vinils vinils left a comment

Choose a reason for hiding this comment

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

still have some peaces of code with the wrong reference (RX.Navigator and RX.Types).
eg.: samples/hello-world/src/App.tsx line 41, 45 and others

@dryganets dryganets force-pushed the sergeyd/remove-navigator-api branch from d3f465f to ff48fce Compare August 24, 2017 19:04
@dryganets dryganets force-pushed the sergeyd/remove-navigator-api branch from ff48fce to 74670f1 Compare August 24, 2017 19:07
@dryganets
Copy link
Contributor Author

@vinils, thank you fixed hello-world and removed the last reference from todo list.

@dryganets dryganets changed the title Navigation API removal Navigation API extracted to the extension Aug 24, 2017
@@ -0,0 +1,143 @@
// Use only for type data
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing copyright header


type NavigationSceneRendererProps = Navigation.NavigationSceneRendererProps;
type NavigationState = Navigation.NavigationState;
type NavigationRoute = Navigation.NavigationRoute;
type NavigationTransitionProps = Navigation.NavigationTransitionProps;

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: extra whitespace

import React = require('react');
import ReactDOM = require('react-dom');
import rebound = require('rebound');

import { NavigatorSceneConfigFactory } from './NavigatorSceneConfigFactory';
import { NavigatorSceneConfig } from './NavigatorSceneConfigFactory';
import RX = require('../common/Interfaces');
import Styles from './Styles';
import RX = require('reactxp');
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: these should go up with other ambient imports

@dryganets dryganets force-pushed the sergeyd/remove-navigator-api branch from acb15f0 to cea94ad Compare August 24, 2017 23:37
@berickson1 berickson1 merged commit e73b2b6 into microsoft:master Aug 24, 2017
@dryganets dryganets deleted the sergeyd/remove-navigator-api branch May 7, 2018 17:30
berickson1 pushed a commit to berickson1/reactxp that referenced this pull request Oct 22, 2018
* Removed Navigation API from reactxp and moved to extension

* Updated samples with reactxp-navigation
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.

4 participants