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

refactor: get rid of setParams #9

Closed
wants to merge 2 commits into from

Conversation

satya164
Copy link
Member

Params are like props to a route and it doesn't make sense for a route to update its own props

Params are like props to a route and it doesn't make sense for a route to update its own props
@satya164 satya164 requested a review from osdnk July 18, 2019 14:54
@codecov-io
Copy link

codecov-io commented Jul 18, 2019

Codecov Report

Merging #9 into master will decrease coverage by 0.79%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master       #9     +/-   ##
=========================================
- Coverage   93.25%   92.45%   -0.8%     
=========================================
  Files          13       13             
  Lines         163      159      -4     
  Branches       35       35             
=========================================
- Hits          152      147      -5     
- Misses         10       11      +1     
  Partials        1        1
Impacted Files Coverage Δ
src/BaseActions.tsx 0% <0%> (ø) ⬆️
src/useOnAction.tsx 95.45% <100%> (ø) ⬆️
src/SceneView.tsx 73.33% <100%> (-6.67%) ⬇️

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 f3c795d...4fd9e18. Read the comment docs.

@satya164 satya164 force-pushed the @satya164/remove-set-params branch 2 times, most recently from 4fd9e18 to e577698 Compare July 18, 2019 17:53
@satya164 satya164 force-pushed the @satya164/remove-set-params branch from e577698 to 7950770 Compare July 18, 2019 17:53
@satya164 satya164 added the WIP label Jul 20, 2019
@satya164 satya164 closed this Jul 20, 2019
@satya164 satya164 deleted the @satya164/remove-set-params branch July 20, 2019 12:22
satya164 pushed a commit that referenced this pull request Aug 18, 2019
Routes in `MaterialTopTabNavigator` are now lazy initialized like in `MaterialBottomTabNavigator`.

A scene visibility is computed from multiple states and props:

To handle the pan between tabs, we check if you're currently swiping between tabs and the prop `lazyOnSwipe` is true (default value) or if the tab have been already loaded, we'll check if this tab is a sibling of the focused tab. Then, we'll display the tab if it's a sibling.

~With the prop `animationEnabled` to true, we shouldn't hide a tab before the transition is done. So we're waiting `COMPLETE_TRANSITION` action to hide it. Also, if the prop `sceneAlwaysVisible` is true (default value), we won't hide scenes between A and D while transitioning.~

If the current tab has not been loaded and must not be visible, we do not render it.

I'll update the docs accordingly to this PR.

![tabs-2](https://user-images.githubusercontent.com/7189823/38261082-3bd30d04-3737-11e8-854e-684430db771f.gif)

<!--
#### Default behavior
Tabs are lazy initialized on swipe or focus and are always visible while transitioning.

![tabs-1](https://user-images.githubusercontent.com/7189823/38260989-060f5808-3737-11e8-87ed-d138fec6022b.gif)

#### Hide tabs between while transitioning

```js
{
  sceneAlwaysVisible: false,
}
```

![tabs-2](https://user-images.githubusercontent.com/7189823/38261082-3bd30d04-3737-11e8-854e-684430db771f.gif)

#### Fallback to only lazy initialized tabs on focus

```js
{
  lazyOnSwipe: false,
}
```

![tabs-3](https://user-images.githubusercontent.com/7189823/38261164-7bcc6018-3737-11e8-8758-de71d28218ae.gif)

-->
satya164 pushed a commit that referenced this pull request Aug 18, 2019
Routes in `MaterialTopTabNavigator` are now lazy initialized like in `MaterialBottomTabNavigator`.

A scene visibility is computed from multiple states and props:

To handle the pan between tabs, we check if you're currently swiping between tabs and the prop `lazyOnSwipe` is true (default value) or if the tab have been already loaded, we'll check if this tab is a sibling of the focused tab. Then, we'll display the tab if it's a sibling.

~With the prop `animationEnabled` to true, we shouldn't hide a tab before the transition is done. So we're waiting `COMPLETE_TRANSITION` action to hide it. Also, if the prop `sceneAlwaysVisible` is true (default value), we won't hide scenes between A and D while transitioning.~

If the current tab has not been loaded and must not be visible, we do not render it.

I'll update the docs accordingly to this PR.

![tabs-2](https://user-images.githubusercontent.com/7189823/38261082-3bd30d04-3737-11e8-854e-684430db771f.gif)

<!--
#### Default behavior
Tabs are lazy initialized on swipe or focus and are always visible while transitioning.

![tabs-1](https://user-images.githubusercontent.com/7189823/38260989-060f5808-3737-11e8-87ed-d138fec6022b.gif)

#### Hide tabs between while transitioning

```js
{
  sceneAlwaysVisible: false,
}
```

![tabs-2](https://user-images.githubusercontent.com/7189823/38261082-3bd30d04-3737-11e8-854e-684430db771f.gif)

#### Fallback to only lazy initialized tabs on focus

```js
{
  lazyOnSwipe: false,
}
```

![tabs-3](https://user-images.githubusercontent.com/7189823/38261164-7bcc6018-3737-11e8-8758-de71d28218ae.gif)

-->
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants