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

fix: Adding ant-upload-btn class to the upload component #1742

Merged
merged 3 commits into from
Feb 1, 2020
Merged

fix: Adding ant-upload-btn class to the upload component #1742

merged 3 commits into from
Feb 1, 2020

Conversation

Neo-Zhixing
Copy link
Contributor

@Neo-Zhixing Neo-Zhixing commented Jan 21, 2020

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?

It was discovered that when the screen is wider than the upload component content, clicking outside of the content bounding box does not trigger the upload dialog. Dragging a file outside of that box causes the browser to open that file instead of uploading the file.

Screen Shot 2020-01-20 at 7 43 59 PM

This bug does not exist on the React version of ant design. After comparing the two versions, it was discovered that the Vue implementation is missing a class on the wrapping span, causing the layout for that wrapper to be broken.

Screen Shot 2020-01-20 at 7 43 31 PM

This PR addresses the issue by adding back the missing class name.

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

@netlify
Copy link

netlify bot commented Jan 21, 2020

Deploy preview for ant-desing-vue processing.

Building with commit 156c3ad

https://app.netlify.com/sites/ant-desing-vue/deploys/5e2deba6631e80000812d0a6

@codecov
Copy link

codecov bot commented Jan 21, 2020

Codecov Report

Merging #1742 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1742   +/-   ##
=======================================
  Coverage   88.09%   88.09%           
=======================================
  Files         157      157           
  Lines        5416     5416           
  Branches     1524     1524           
=======================================
  Hits         4771     4771           
  Misses        576      576           
  Partials       69       69
Impacted Files Coverage Δ
components/upload/Upload.jsx 84.15% <ø> (ø) ⬆️

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 b8cd2b5...156c3ad. Read the comment docs.

@Neo-Zhixing Neo-Zhixing changed the title Adding ant-upload-btn class to the upload component fix: Adding ant-upload-btn class to the upload component Jan 21, 2020
Copy link
Member

@tangjinzhou tangjinzhou left a comment

Choose a reason for hiding this comment

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

you should change this line to <VcUpload {...vcUploadProps} class={${prefixCls}-btn}>
https://github.com/vueComponent/ant-design-vue/blob/master/components/upload/Upload.jsx#L276

@Neo-Zhixing
Copy link
Contributor Author

The requested changes have been made.

@tangjinzhou tangjinzhou merged commit 2a304e0 into vueComponent:master Feb 1, 2020
@github-actions
Copy link

github-actions bot commented Feb 1, 2021

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 Feb 1, 2021
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.

2 participants