Skip to content

Conversation

@hvsy
Copy link
Contributor

@hvsy hvsy commented Jun 28, 2019

@vercel
Copy link

vercel bot commented Jun 28, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://rc-field-form-git-fork-hvsy-master.react-component.now.sh

@zombieJ
Copy link
Member

zombieJ commented Jul 1, 2019

@hvsy

@zombieJ
Copy link
Member

zombieJ commented Jul 2, 2019

Could you help to create a new example named list-draggable instead?
And also, I test the example find that text can not be selectable:
Kapture 2019-07-02 at 18 15 48

Add list-draggable example
@zombieJ
Copy link
Member

zombieJ commented Jul 3, 2019

CI failed. Please check.

@codecov
Copy link

codecov bot commented Jul 3, 2019

Codecov Report

Merging #20 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #20      +/-   ##
==========================================
+ Coverage   99.85%   99.86%   +<.01%     
==========================================
  Files          13       13              
  Lines         698      720      +22     
  Branches      138      152      +14     
==========================================
+ Hits          697      719      +22     
  Misses          1        1
Impacted Files Coverage Δ
src/List.tsx 100% <100%> (ø) ⬆️
src/utils/valueUtil.ts 100% <100%> (ø) ⬆️

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 605bffe...2e99658. Read the comment docs.

@zombieJ
Copy link
Member

zombieJ commented Jul 3, 2019

sample seems laggy with user operation and drag not perf correctly:
Kapture 2019-07-03 at 18 01 52

remove console log for performance
@hvsy
Copy link
Contributor Author

hvsy commented Jul 3, 2019

@zombieJ after remove console log and seems to work well so far.
QQ20190703-211727-HD

rename "arrayMove" to "move".
add checking for out of range.
add test cases for out of range.
merge edge case check into one if.
@hvsy
Copy link
Contributor Author

hvsy commented Jul 15, 2019

@zombieJ
Are there any suggestions for improvement?


return (
<DndProvider backend={HTML5Backend}>
<div>
Copy link
Member

Choose a reason for hiding this comment

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

miss 2 space as of child node.

const { List, useForm } = Form;

type LabelFieldProps = Parameters<typeof LabelField>[0];
interface DraggableProps extends LabelFieldProps{
Copy link
Member

Choose a reason for hiding this comment

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

LabelFieldProps{ => LabelFieldProps {

Seems still missing types. Could you try to remove .git/hooks and reinstall node_modules to make father pre-commit hooks work. This will auto prettier the code when commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please reference to following code. The default config don't include the examples directory.
so father pre-commit don't work for it.

https://github.com/umijs/father/blob/86e4eb154c212072cd937d6a87fd91762159e069/packages/father/src/preCommit.ts#L113

Copy link
Member

Choose a reason for hiding this comment

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

Let me add PR for this. Thanks for notice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I add "examples" to the regex of pre-commit by manual. after then, I get following errors by exec "father pre-commit".

'react-dnd-html5-backend' and 'react-dnd' should only be used for example. it should be listed in devDependencies.

image

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for feedback. Add PR for this: umijs/fabric#6

Copy link
Member

Choose a reason for hiding this comment

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

Updated @umi/fabric, please check.

format draggable example
@zombieJ zombieJ changed the title add move operation for List feat: add move operation for List Jul 16, 2019
@zombieJ zombieJ merged commit 0f48380 into react-component:master Jul 16, 2019
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.

2 participants