Typescript/tab controller move to typescript #1079

M-i-k-e-l · 2020-12-16T14:06:43Z

Description

TabController - move to typescript
This is step one from 3 (step 2 is move to functional component and 3 is to add a presenter for the centering).
There are a few TODOs for typescript, some will be removed once the move to functional component is done, but I'm not sure all will be removed, especially the ones that are related to reanimated, they don't export all of their types.

Changelog

TabController - move to typescript

ethanshar · 2020-12-16T19:03:26Z

src/index.ts

@@ -36,6 +36,7 @@ export {default as Overlay, OverlayTypes} from './components/overlay';
 export {default as RadioButton, RadioButtonPropTypes, RadioButtonProps} from './components/radioButton/RadioButton';
 export {default as RadioGroup, RadioGroupPropTypes, RadioGroupProps} from './components/radioButton/RadioGroup';
 export {default as Switch, SwitchProps} from './components/switch';
+export {default as TabController, TabControllerProps, TabBarItemProps} from './components/tabController';


TabBarItemProps might conflict in the future with our TabBar component which will probably have also TabBatItemProps :/
Maybe we can name this one TabControllerItemProps, WDYT?

I think it might get confusing since all the sub components there are (for TabController), and I'm not sure people won't need their props as well.
Maybe we can add them statically? Either on the component or create another "MegaProp" that has them all?
So either TabController.Props, TabController.TabBar.Props... or TabControllerProps.Main, TabControllerProps.TabBar...
Not sure how easy it is to do or understand, WDYT?

I'm not sure you can create this type of nesting in typings...
If it's possible then let's do it..

I guess that's the reason other libraries' typings have weird long names..

I went with TabControllerProps namespace, let me know if that's ok

The namespace is nice, but now users will need to do this?

TabControllerProps.TabControllerProps? it's a little weird don't you think?

:s

Any suggestions? TabControllerProps.Props or maybe create the namespace as TabController and everything will be inside it (but the it'll be TabController.TabController and worse TabController.TabController.TabBar).

TBH, Im starting to think we should drop the namespace thing, it's a little messy and too nested.
Im not sure people are used to import typings this way.
I prefer to have
TabControllerProps
TabControllerBarProps
TabControllerItemProps
TabControllerPageProps, etc...

It's not the best, but feels simpler.. WDYT?

I agree, changed as you recommended

ethanshar

Overall look good.
Wrote some comments.

Anyway, i Noticed some performance issues with the component, but I think it has nothing to do with your PR.
Recently, we update reanimated version to v13.1 and I noticed some weird issues in TabController.
Do you experience them as well?

src/components/tabController/PageCarousel.tsx

src/components/tabController/TabBar.tsx

src/components/tabController/TabBarItem.tsx

src/components/tabController/TabPage.tsx

src/components/tabController/index.tsx

M-i-k-e-l · 2020-12-16T19:40:02Z

Overall look good.
Wrote some comments.

Anyway, i Noticed some performance issues with the component, but I think it has nothing to do with your PR.
Recently, we update reanimated version to v13.1 and I noticed some weird issues in TabController.
Do you experience them as well?

Yes, I think I know what you mean, it also sometimes does not go to the correct page. I was thinking that it might be solved with the new logic 🤞
Edit: looking at it again, not sure the new logic will fix it since it happens not only in centered mode.

ethanshar · 2020-12-17T06:09:54Z

Yes, I think I know what you mean, it also sometimes does not go to the correct page. I was thinking that it might be solved with the new logic 🤞
Edit: looking at it again, not sure the new logic will fix it since it happens not only in centered mode.

Ok.. so I just verified.
On react-native-reanimated@1.9.0 (the previous version we had), everything works great
After updating to react-native-reanimated@1.13.1 (which was required for something else) things started to go south..

So I suggest that after we merge this PR, We'll investigate the issue first before going forward.
Usually when there are issues because of version upgrade, it's probably something small we need to tweak. 🤞

M-i-k-e-l · 2020-12-17T10:52:02Z

Yes, I think I know what you mean, it also sometimes does not go to the correct page. I was thinking that it might be solved with the new logic 🤞
Edit: looking at it again, not sure the new logic will fix it since it happens not only in centered mode.

Ok.. so I just verified.
On react-native-reanimated@1.9.0 (the previous version we had), everything works great
After updating to react-native-reanimated@1.13.1 (which was required for something else) things started to go south..

So I suggest that after we merge this PR, We'll investigate the issue first before going forward.
Usually when there are issues because of version upgrade, it's probably something small we need to tweak. 🤞

ok

src/components/tabController/index.tsx

* Fix first tab item change doesn't repsond * Memoize renderCodeBlock in TabController components

* master: configure 'max-len'; turn off 'jsx-no-bind' (#1087) remove unnecessary view (#1084) Counter Icon badge (#1081) checkbox variant - outline (#1067)

This reverts commit 0d3cff5.

This reverts commit cd52f05.

This reverts commit 2b493ee.

* master: Fix/avatars label alignment (#1082)

M-i-k-e-l added 22 commits December 15, 2020 14:05

Move TabBarContext from js ro ts

885f586

Move TabPage from js to tsx

d12ccc4

TabPage - move to typescript (fix typescript)

0405166

Move PageCarousel from js to tsx

982d177

PageCarousel - move to typescript (fix typescript)

1249e2e

Fix TabPage exception

e7c2464

Delete ReanimatedObject

9836261

Move TabBarItem from js to tsx

0c2c3ea

export TabPageProps

0314f98

TabPage - fix description

3a2af83

Commit .d files

1beb39c

TabBarItem - move to typescript (fix typescript)

b2199d3

PageCarousel - move to typescript (fix typescript) - part 2

feac3cd

Move TabBar from js to tsx

c1d257c

Move ScrollBarGradient from js to tsx

1086511

ScrollBarGradient add props and docs

3fa5940

Commit .d file

0929830

TabBar - move to typescript (fix typescript)

1e677af

Move TabBarController from js to tsx

8239fdb

TabController - move to typescript (fix typescript)

59f7572

Move TabControllerScreen to ts

89def2a

Fix screen and some errors

94c3c69

M-i-k-e-l requested a review from ethanshar December 16, 2020 14:06

M-i-k-e-l assigned ethanshar Dec 16, 2020

ethanshar reviewed Dec 16, 2020

View reviewed changes

M-i-k-e-l added 2 commits December 16, 2020 23:33

Fix PR comments

17d0e0a

Fix prop

4cbab00

M-i-k-e-l requested a review from ethanshar December 16, 2020 21:38

ethanshar reviewed Dec 20, 2020

View reviewed changes

src/components/tabController/index.tsx Outdated Show resolved Hide resolved

ethanshar and others added 4 commits December 20, 2020 15:31

Fix/tab controller transition (#1089)

b747c91

* Fix first tab item change doesn't repsond * Memoize renderCodeBlock in TabController components

Merge branch 'master' into typescript/tab-controller-move-to-typescript

e6975e2

* master: configure 'max-len'; turn off 'jsx-no-bind' (#1087) remove unnecessary view (#1084) Counter Icon badge (#1081) checkbox variant - outline (#1067)

Move TabBarController from index.tsx to TabBarController.tsx

2b493ee

Create TabControllerProps namespace

cd52f05

M-i-k-e-l requested a review from ethanshar December 20, 2020 13:40

M-i-k-e-l added 6 commits December 20, 2020 17:58

Create TabControllerProps namespace (fix screen)

0d3cff5

Revert "Create TabControllerProps namespace (fix screen)"

ede0f36

This reverts commit 0d3cff5.

Revert "Create TabControllerProps namespace"

2feb180

This reverts commit cd52f05.

Revert "Move TabBarController from index.tsx to TabBarController.tsx"

0178216

This reverts commit 2b493ee.

Change interfaces names for TabController related classes

d16185d

Merge branch 'master' into typescript/tab-controller-move-to-typescript

7e37a8c

* master: Fix/avatars label alignment (#1082)

ethanshar approved these changes Dec 22, 2020

View reviewed changes

ethanshar merged commit 58340ed into master Dec 22, 2020

M-i-k-e-l deleted the typescript/tab-controller-move-to-typescript branch January 25, 2021 09:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typescript/tab controller move to typescript #1079

Typescript/tab controller move to typescript #1079

M-i-k-e-l commented Dec 16, 2020

ethanshar Dec 16, 2020

M-i-k-e-l Dec 16, 2020

ethanshar Dec 17, 2020

M-i-k-e-l Dec 20, 2020

ethanshar Dec 20, 2020

M-i-k-e-l Dec 20, 2020

ethanshar Dec 21, 2020

M-i-k-e-l Dec 21, 2020

ethanshar left a comment

M-i-k-e-l commented Dec 16, 2020 •

edited

ethanshar commented Dec 17, 2020

M-i-k-e-l commented Dec 17, 2020

Typescript/tab controller move to typescript #1079

Typescript/tab controller move to typescript #1079

Conversation

M-i-k-e-l commented Dec 16, 2020

Description

Changelog

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethanshar left a comment

Choose a reason for hiding this comment

M-i-k-e-l commented Dec 16, 2020 • edited

ethanshar commented Dec 17, 2020

M-i-k-e-l commented Dec 17, 2020

M-i-k-e-l commented Dec 16, 2020 •

edited