Skip to content
This repository has been archived by the owner on Feb 8, 2020. It is now read-only.

feat: make NAVIGATE and JUMP_TO to support key and name of the route #16

Merged
merged 3 commits into from
Jul 20, 2019

Conversation

osdnk
Copy link
Member

@osdnk osdnk commented Jul 20, 2019

No description provided.

@codecov-io
Copy link

codecov-io commented Jul 20, 2019

Codecov Report

Merging #16 into master will decrease coverage by 0.79%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #16     +/-   ##
=========================================
- Coverage   94.88%   94.09%   -0.8%     
=========================================
  Files          14       14             
  Lines         215      220      +5     
  Branches       44       49      +5     
=========================================
+ Hits          204      207      +3     
- Misses          9       11      +2     
  Partials        2        2
Impacted Files Coverage Δ
src/BaseActions.tsx 33.33% <50%> (+33.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10708da...1782c0b. Read the comment docs.

@@ -151,25 +151,38 @@ const StackRouter: Router<CommonAction | Action> = {
});

case 'NAVIGATE':
if (state.routeNames.includes(action.payload.name)) {
if (
action.payload.key ||
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should also check if the key exists in the stack

Copy link
Member Author

Choose a reason for hiding this comment

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

we do it line 180

@@ -60,6 +60,76 @@ it('initializes state for a navigator on navigation', () => {
});
});

it('throws if NAVIGATE dispatched with both key and name', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Let's move these to a separate file

(target.hasOwnProperty('key') && target.hasOwnProperty('name')) ||
(!target.hasOwnProperty('key') && !target.hasOwnProperty('name'))
) {
throw new Error(
Copy link
Member

Choose a reason for hiding this comment

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

Maybe instead of throwing in the action creator, we can throw in the router. Then it'll also prevent someone from calling dispatch directly with incorrect stuff

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. We don't want anyone to dispatch NAVIGATE directly. If yes, risk included. I don't want to copy this error to every new and existing navigators.

@@ -19,8 +19,20 @@ export function goBack(): Action {
return { type: 'GO_BACK' };
}

export function navigate(name: string, params?: object): Action {
return { type: 'NAVIGATE', payload: { name, params } };
export function navigate(target: TargetForAction, params?: object): Action {
Copy link
Member

Choose a reason for hiding this comment

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

We also need to change the types in the NavigationProp object in types.tsx

@@ -171,8 +181,20 @@ const TabRouter: Router<Action | CommonAction> = {
},

actionCreators: {
jumpTo(name: string, params?: object) {
return { type: 'JUMP_TO', payload: { name, params } };
jumpTo(target: TargetForAction, params?: object) {
Copy link
Member

Choose a reason for hiding this comment

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

We also need to change types for the TabNavigationProp type above

src/types.tsx Outdated
@@ -2,6 +2,8 @@ import * as BaseActions from './BaseActions';

export type CommonAction = BaseActions.Action;

export type TargetForAction = string | { name: string } | { key: string };
Copy link
Member

Choose a reason for hiding this comment

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

Maybe call it TargetRoute?

@osdnk osdnk merged commit 63290fb into master Jul 20, 2019
@osdnk osdnk deleted the @osdnk/base-action-target-object branch July 20, 2019 15:46
satya164 added a commit that referenced this pull request Aug 18, 2019
Closes #16

When the statusbar is not translucent, the view resizes when the keyboard is shown on Android. The tab bar stays above the keyboard. This PR makes the tab bar hide automatically when the keyboard is shown.

The behaviour is enabled by default and can be disabled with `keyboardHidesTabBar: false` in `tabBarOptions`
satya164 added a commit that referenced this pull request Aug 18, 2019
Closes #16

When the statusbar is not translucent, the view resizes when the keyboard is shown on Android. The tab bar stays above the keyboard. This PR makes the tab bar hide automatically when the keyboard is shown.

The behaviour is enabled by default and can be disabled with `keyboardHidesTabBar: false` in `tabBarOptions`
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants