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.

Sign up for GitHub

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

Jump to bottom

fix(Modal): prompt should handle defaultValue correctly #2221

Merged
merged 2 commits into from
Jan 22, 2018
Merged

fix(Modal): prompt should handle defaultValue correctly #2221

merged 2 commits into from
Jan 22, 2018

Conversation

paranoidjk
Copy link
Contributor

@paranoidjk paranoidjk commented Jan 5, 2018 •
edited
Loading

Fix #2164
Fix #2204

First of all, thank you for your contribution! :-)

Please makes sure that these checkboxes are checked before submitting your PR, thank you!

  • Make sure that you follow antd's code convention.
  • Run npm run lint and fix those errors before submitting in order to keep consistent code style.
  • Rebase before creating a PR to keep commit history clear.
  • Add some descriptions and refer relative issues for you PR.

Extra checklist:

if isBugFix :

  • Make sure that you add at least one unit test for the bug which you had fixed.

elif isNewFeature :

  • Update API docs for the component.
  • Update/Add demo to demonstrate new feature.
  • Update TypeScript definition for the component.
  • Add unit tests for the feature.

This change is Reviewable

@paranoidjk paranoidjk requested a review from silentcloud January 5, 2018 07:22
@ghost ghost assigned paranoidjk Jan 5, 2018
@ghost ghost added the In Progress label Jan 5, 2018
@paranoidjk paranoidjk removed their assignment Jan 5, 2018
@warmhug
Copy link
Contributor

warmhug commented Jan 9, 2018

问题还没有解决。看下这里 0d2981d#commitcomment-26734191

@warmhug
Copy link
Contributor

warmhug commented Jan 9, 2018

#2204 这个也应该是有问题的，一起看看

@paranoidjk paranoidjk changed the title fix(Modal): prompt should always reflect user's input WIP fix(Modal): prompt should always reflect user's input Jan 22, 2018
@ghost ghost assigned paranoidjk Jan 22, 2018
@paranoidjk paranoidjk changed the title WIP fix(Modal): prompt should always reflect user's input fix(Modal): prompt should always reflect user's input Jan 22, 2018
@paranoidjk
Copy link
Contributor Author

@warmhug 好了

@paranoidjk
Copy link
Contributor Author

@silentcloud @zhang740 也可以看下，这里比较特殊，Modal 内部的 Input 不会提供 onChange 给用户，只有一个类似 onConfirm, onSubmit 的最终回调

@paranoidjk paranoidjk changed the title fix(Modal): prompt should always reflect user's input fix(Modal): prompt should handle defaultValue correctly Jan 22, 2018
@paranoidjk paranoidjk removed their assignment Jan 22, 2018
paranoidjk
paranoidjk commented Jan 22, 2018
View reviewed changes
components/modal/prompt.tsx
@@ -18,7 +17,9 @@ export default function prompt(

const prefixCls = 'am-modal';

let data: any = {};
let data: any = {
text: defaultValue,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

readme API 文档 里面写 password 不支持设置 defaultValue 的设计小版本就不改了，以后如果有用户有需求再看

@silentcloud
Copy link
Contributor

我记得之前 #2204 是修过的，怎么又报出来了

@paranoidjk
Copy link
Contributor Author

这两个 bug 都是同一类问题，没处理好 defaultValue => value => onChange(value) 的处理

@paranoidjk paranoidjk changed the title fix(Modal): prompt should handle defaultValue correctly WIP fix(Modal): prompt should handle defaultValue correctly Jan 22, 2018
@ghost ghost assigned paranoidjk Jan 22, 2018
@codecov
Copy link

codecov bot commented Jan 22, 2018 •
edited
Loading

Codecov Report

Merging #2221 into master will increase coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2221      +/-   ##
==========================================
+ Coverage   57.69%   57.72%   +0.03%     
==========================================
  Files         260      260              
  Lines        4460     4457       -3     
  Branches     1178     1176       -2     
==========================================
  Hits         2573     2573              
+ Misses       1886     1883       -3     
  Partials        1        1
Flag Coverage Δ
#rn 60.78% <ø> (ø) ⬆️
#web 55.11% <0%> (+0.06%) ⬆️
Impacted Files Coverage Δ
components/modal/prompt.tsx 0% <0%> (ø) ⬆️

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 07a41c5...473ba42. Read the comment docs.

@paranoidjk paranoidjk changed the title WIP fix(Modal): prompt should handle defaultValue correctly fix(Modal): prompt should handle defaultValue correctly Jan 22, 2018
@warmhug warmhug merged commit 20d8eb3 into master Jan 22, 2018
@warmhug warmhug deleted the fix-modal-prompt branch January 22, 2018 07:45
lixiaoyang1992 pushed a commit to lixiaoyang1992/ant-design-mobile that referenced this pull request Apr 26, 2018
@paranoidjk @lixiaoyang1992
fix(Modal): prompt should handle defaultValue correctly (ant-design#2221
d80d43c 
)

* fix(Modal): Modal.prompt defaultValue. close ant-design#2164 close ant-design#2204

* chrore: code style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@silentcloud silentcloud Awaiting requested review from silentcloud

Assignees

@warmhug warmhug

@paranoidjk paranoidjk

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@paranoidjk @warmhug @silentcloud