Skip to content

fix(Image): can not cancel previewMask #5531

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

Merged
merged 4 commits into from
Apr 22, 2022

Conversation

buqiyuan
Copy link
Member

@buqiyuan buqiyuan commented Apr 20, 2022

First of all, thank you for your contribution! 😄

fix #5524

New feature please send pull request to feature branch, and rest to master branch. Pull request will be merged after one of collaborators approve. Please makes sure that these form are filled before submitting your pull request, thank you!

[中文版模板 / Chinese template]

This is a ...

  • New feature
  • Bug fix
  • Site / document update
  • Component style update
  • TypeScript definition update
  • Refactoring
  • Code style optimization
  • Branch merge
  • Other (about what?)

What's the background?

  1. Describe the source of requirement.
  2. Resolve what problem.
  3. Related issue link.

API Realization (Optional if not new feature)

  1. Basic thought of solution and other optional proposal.
  2. List final API realization and usage sample.
  3. GIF or snapshot should be provided if includes UI/interactive modification.

What's the effect? (Optional if not new feature)

  1. Does this PR affect user? Which part will be affected?
  2. What will say in changelog?
  3. Does this PR contains potential break change or other risk?

Changelog description (Optional if not new feature)

  1. English description
  2. Chinese description (optional)

Self Check before Merge

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

Additional Plan? (Optional if not new feature)

If this PR related with other PR or following info. You can type here.

@buqiyuan buqiyuan force-pushed the fix/image-previewMask branch from e9620df to 29443ae Compare April 20, 2022 11:57
@tangjinzhou
Copy link
Member

忘了说我的想法了:
我们不用支持吧,给他个方案就可以了 不是有 maskClassName 这个了吗,可以满足他了

@buqiyuan
Copy link
Member Author

buqiyuan commented Apr 21, 2022

忘了说我的想法了: 我们不用支持吧,给他个方案就可以了 不是有 maskClassName 这个了吗,可以满足他了

😂自定义maskClassName然后再设置display: none;吗?但这样感觉可能就没有将previewMask设置为false那么符合用户的直观,并且我也体验了一下vc-image
image
在上面这个逻辑中,previewMask的值为falsy的时候都不会渲染mask,由于我们的previewMaskslot所以始终为truthy。所以我觉得这样也符合vc-image原本的一个逻辑,previewMask也相对于maskClassName更语义化一些,并且做这个支持的成本也不高,我也顺便移除了一些多余的代码

@buqiyuan buqiyuan force-pushed the fix/image-previewMask branch from 6a4b40d to e5b8221 Compare April 22, 2022 05:22
@buqiyuan buqiyuan force-pushed the fix/image-previewMask branch from c699b39 to 70fb553 Compare April 22, 2022 05:50
@tangjinzhou tangjinzhou merged commit bc3843b into vueComponent:main Apr 22, 2022
tangjinzhou added a commit that referenced this pull request Apr 22, 2022
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image component, close the mask and its content when hovering
2 participants