-
-
Notifications
You must be signed in to change notification settings - Fork 235
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
chore: rm mixin and react-create-class #123
Conversation
src/InkTabBar.js
Outdated
this[name] = node; | ||
} | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这些也可以抽出来:
export default function SwipeableInkTabBar() {
return (
<SaveRef>
{({ save, get }) => (
<TabBarRootNode saveRef={save} {...this.props}>
<SwipeableTabBarNode saveRef={save} getRef={get} {...this.props}>
<InkTabBarNode saveRef={save} getRef={get} {...this.props} />
<TabBarSwipeableTabs saveRef={save} {...this.props} />
</SwipeableTabBarNode>
</TabBarRootNode>
)}
<SaveRef>
);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
可能要先把覆盖率提升下。 |
找到问题了,原来用的是 mixin , 生命周期都在同一个组件上,现在的模式把生命周期拆分了。InkTabBarNode 的 componenentDidUpdate 需要在 TabBarTabsNode 后面发生。 加了一个测试用例。再看下 |
前面那个有点奇怪,后面这个我晚上看一下 |
@yesmeck 看过了 diff 是无害的, 一种是 inkbar 和 tabbar 顺序反了,这是因为 inkbar 依赖了 tabbar 的状态,所以需要把 inkbar 放在 tabbar 后面渲染,也就是这里的 https://github.com/react-component/tabs/pull/123/files#diff-ad3456d46a980c23853783634a1555a7R127 另外上面这个图的 diff 就是多了一堆中间状态节点,也是预期之中的 |
需要 rebase 下,把这个问题 #121 在这个分支上也修下。 |
InkTabBarNode has to be after TabBarTabsNode, since InkTabBarNode depends on current activeTab which is set inside TabBarTabsNode
rebase 完毕,看了下新的 diff 没啥问题,我合了! |
Todo:
rm componentWillReceiveProps and componentWillUnMount.
Better do in a seperate PR;