Skip to content
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

improve: preview props #34

Merged
merged 4 commits into from Oct 14, 2020
Merged

improve: preview props #34

merged 4 commits into from Oct 14, 2020

Conversation

hengkx
Copy link
Member

@hengkx hengkx commented Oct 13, 2020

@vercel
Copy link

vercel bot commented Oct 13, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/react-component/image/kba5rz71a
✅ Preview: https://image-git-refactor-container.react-component.vercel.app

@codecov
Copy link

codecov bot commented Oct 13, 2020

Codecov Report

Merging #34 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #34   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          143       143           
  Branches        35        36    +1     
=========================================
  Hits           143       143           
Impacted Files Coverage Δ
src/Image.tsx 100.00% <100.00%> (ø)

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 2a164d7...20abbbc. Read the comment docs.

@shaodahong
Copy link
Member

What is the difference with https://github.com/react-component/image/pull/29?

@hengkx
Copy link
Member Author

hengkx commented Oct 13, 2020

预览的参数 改到了 preview里面

@hengkx
Copy link
Member Author

hengkx commented Oct 13, 2020

image

src/Image.tsx Show resolved Hide resolved
src/Image.tsx Outdated Show resolved Hide resolved
Copy link
Member

@afc163 afc163 left a comment

Choose a reason for hiding this comment

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

要发个 major 版本。

@hengkx hengkx requested a review from zombieJ October 13, 2020 05:51
@shaodahong
Copy link
Member

测试失败是这个 react-component/dialog#206 导致的,以前是元素直接干掉了,现在是 display: none 要改下测试

@shaodahong
Copy link
Member

是否有必要给dialog 也加一个用例?

感觉底层的库还是在底层测试比较好

@shaodahong shaodahong merged commit ce47c04 into master Oct 14, 2020
@shaodahong shaodahong deleted the refactor/container branch October 14, 2020 09:52
@hengkx
Copy link
Member Author

hengkx commented Oct 14, 2020

passed 🤗

@shaodahong
Copy link
Member

shaodahong commented Oct 14, 2020

+ rc-image@3.3.0

@afc163
Copy link
Member

afc163 commented Oct 14, 2020

It is a breaking change, please revert and release a major version.

@shaodahong
Copy link
Member

不算 breaking change 吧,原本的 getPopupContainer 没发布 ant-design/ant-design#26713

@hengkx
Copy link
Member Author

hengkx commented Oct 14, 2020

应该算是 rc-image 已经发布过了 对他来说 已经是 breaking change 了吧

@afc163
Copy link
Member

afc163 commented Oct 14, 2020

对 antd 来说没有这个属性,对于 rc-image 发布了。

@yoyo837
Copy link
Member

yoyo837 commented Oct 14, 2020

老偏的意思是,有的用户不是通过antd而使用rc-image的

@shaodahong
Copy link
Member

shaodahong commented Oct 14, 2020

+ rc-image@4.0.0

@yoyo837
Copy link
Member

yoyo837 commented Oct 14, 2020

是不是还应该发个3.3.1,把这个revert发布出去

@hengkx
Copy link
Member Author

hengkx commented Oct 14, 2020

npm 在一定时间内 可以撤销的

@yoyo837
Copy link
Member

yoyo837 commented Oct 14, 2020

我是看见npm里有才说的!

那...撤销一下吧

@shaodahong
Copy link
Member

撤销不了,已经 deprecate

rfreling added a commit to rfreling/ant-design that referenced this pull request Oct 15, 2020
@rfreling
Copy link
Contributor

谢谢你们! 🙏 @afc163 @shaodahong @hengkx @yoyo837

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.

None yet

6 participants