Skip to content

[WIP]Preact compat#76

Closed
pingan1927 wants to merge 5 commits intomasterfrom
preact-compat
Closed

[WIP]Preact compat#76
pingan1927 wants to merge 5 commits intomasterfrom
preact-compat

Conversation

@pingan1927
Copy link
Copy Markdown
Contributor

lint & test done.

原来写法preact会有问题,activeKey获取的不是最新值。

改了ref的写法,看看是否需要发大版本。

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling fb52de7 on preact-compat into ** on master**.

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 32e3224 on preact-compat into ** on master**.

@pingan1927 pingan1927 requested a review from paranoidjk July 12, 2017 02:21
@paranoidjk
Copy link
Copy Markdown
Member

ref #77 一并修复下算了 😄

@pingan1927
Copy link
Copy Markdown
Contributor Author

done. 那个pr是外国友人提交的?

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 6b08174 on preact-compat into ** on master**.

}
const key = child.key;
let cls = activeKey === key ? `${prefixCls}-tab-active` : '';
let cls = this.props.activeKey === key ? `${prefixCls}-tab-active` : '';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这个改动是为什么?应该没有区别?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

事实是有区别的,搞了我一天,fml

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

不过这行代码可能没影响,但这个方法体内要改就全改了。

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

为什么... 分享下踩的坑

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

不清楚原因的改动没法合的

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

const containerOffset = offset(wrapNode);
const inkBarNode = refs.inkBar;
const activeTab = refs.activeTab;
const activeTab = component.activeTab;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这个改动,rc-tabs 肯定是要升 major 的

不确定是不是有依赖 refs.activeTab

@paranoidjk
Copy link
Copy Markdown
Member

close #77
@pingan1927 养成在 commit message 和 pr description 里面自动关闭对应issue的习惯 😄

see https://help.github.com/articles/closing-issues-via-commit-messages/

@paranoidjk paranoidjk self-assigned this Jul 13, 2017
@pingan1927 pingan1927 mentioned this pull request Jul 14, 2017
key={key}
{...refProps}
ref={(tab) => {
if (this.props.activeKey === key) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

关键的一点:外层定义的const activeKey = props.activeKey;在ref的callback执行的时候,不是预期中的值,已经变掉了。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

直接取this.props.activeKey 就符合预期了

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这怎么可能? 你的意思是用户把 activeKey 改变了?callback 执行的时候拿到的是旧的值?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

运行demo, 把react 依赖换成preact配套的依赖,就可以重现了

@paranoidjk
Copy link
Copy Markdown
Member

@pingan1927 rebase master, 我加了个 npm run start:preact, 在你这个分支 npm start 正常npm run start:preact 切换的时候 tabbar 显示不正常,你再检查下

@paranoidjk paranoidjk changed the title Preact compat [WIP]Preact compat Jul 28, 2017
@paranoidjk
Copy link
Copy Markdown
Member

paranoidjk commented Jul 28, 2017

@pingan1927 rebase master, i fixed #77

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.2%) to 70.019% when pulling 9f17593 on preact-compat into 242b577 on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.2%) to 70.019% when pulling 9f17593 on preact-compat into 242b577 on master.

@pingan1927
Copy link
Copy Markdown
Contributor Author

pingan1927 commented Jul 28, 2017

@paranoidjk done, test应该都是通过了的

Copy link
Copy Markdown
Member

@paranoidjk paranoidjk left a comment

Choose a reason for hiding this comment

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

@pingan1927 测试要仔细一点。用真机测,在 preact 上,滑动 tab 内容切换 tab 失效了,切换回 react 没问题。

@pingan1927
Copy link
Copy Markdown
Contributor Author

哦?这两天忙完了 继续看看

@paranoidjk
Copy link
Copy Markdown
Member

@pingan1927 这个拖太久了,再不处理我就过期关掉了?

@paranoidjk
Copy link
Copy Markdown
Member

这个我来处理算了。

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-1.0%) to 67.838% when pulling 22a216a on preact-compat into 8ab8bab on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.9%) to 67.958% when pulling 22a216a on preact-compat into 8ab8bab on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.9%) to 67.958% when pulling 9423f17 on preact-compat into 8ab8bab on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-1.3%) to 67.483% when pulling 0ea9baf on preact-compat into 949c4d3 on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-1.3%) to 67.483% when pulling e747eab on preact-compat into 949c4d3 on master.

@paranoidjk
Copy link
Copy Markdown
Member

close. @zhang740 在重写 rmc-tabs. @zhang740 开发过程中可以用我 fork 的 rmc-tools https://github.com/paranoidjk/rc-tools#preact 测试下 preact 运行是否ok。

@paranoidjk paranoidjk closed this Sep 1, 2017
@paranoidjk paranoidjk deleted the preact-compat branch September 1, 2017 02:29
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.

3 participants