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

feat(data-table): support shiftKey check #1026

Merged

Conversation

doom-9-zz
Copy link
Contributor

@doom-9-zz doom-9-zz commented Aug 31, 2021

doom-9 and others added 30 commits June 17, 2021 17:42
@vercel
Copy link

vercel bot commented Aug 31, 2021

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/tusimple/naive-ui/3cSX9ZT2hRib5VxXvoQUEe2orqnP
✅ Preview: https://naive-ui-git-fork-doom-9-featdata-table-support-5b20a1-tusimple.vercel.app

@codecov
Copy link

codecov bot commented Aug 31, 2021

Codecov Report

Merging #1026 (0d6c633) into main (174377e) will decrease coverage by 0.07%.
The diff coverage is 11.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1026      +/-   ##
==========================================
- Coverage   47.14%   47.07%   -0.08%     
==========================================
  Files         511      511              
  Lines       12590    12612      +22     
  Branches     3547     3551       +4     
==========================================
+ Hits         5936     5937       +1     
- Misses       5628     5649      +21     
  Partials     1026     1026              
Impacted Files Coverage Δ
src/data-table/src/TableParts/BodyCheckbox.tsx 0.00% <ø> (ø)
src/data-table/src/interface.ts 100.00% <ø> (ø)
src/data-table/src/use-check.ts 50.87% <ø> (ø)
src/data-table/src/TableParts/Body.tsx 41.80% <5.26%> (-4.11%) ⬇️
src/checkbox/src/Checkbox.tsx 70.66% <25.00%> (-3.99%) ⬇️

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 174377e...0d6c633. Read the comment docs.

@@ -51,7 +51,7 @@ const checkboxProps = {
MaybeArray<(value: boolean) => void>
>,
onUpdateChecked: [Function, Array] as PropType<
MaybeArray<(value: boolean) => void>
MaybeArray<(value: boolean, e?: MouseEvent | KeyboardEvent) => void>
Copy link
Collaborator

Choose a reason for hiding this comment

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

什么时候会没有事件?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我的想法是,用户传函数进来时,e参数可以不用写

Copy link
Collaborator

Choose a reason for hiding this comment

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

你不写这个 ? 用户也可以传 () => {} 进来。

一个组件接受一个回调的参数意义为“组件会把声明的参数传进来”,用户少写参数无所谓,也就是说你写了 (value, e) 则组件会向回调传入 (value, e),用户传进来 () => {}` 也无所谓,这种情况下只是用户不需要参数,但是组件会传进来。

@@ -289,6 +289,7 @@ export default defineComponent({
onKeyup={handleKeyUp}
onKeydown={handleKeyDown}
onClick={handleClick}
onMousedown={(e) => e.preventDefault()}
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个不行啊,很危险

这个会影响 focus 状态

@@ -137,10 +139,34 @@ export default defineComponent({
} = inject(dataTableInjectionKey)!
const scrollbarInstRef = ref<ScrollbarInst | null>(null)
const virtualListRef = ref<VirtualListInst | null>(null)
const lastSelectedIndex = ref<number | null>(null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

其实我觉得可以用 lastSelectedKey 去做,这样的话不用每次点击都遍历一遍 pageData

顺便一提,这个并非响应式,不需要 ref

@07akioni
Copy link
Collaborator

07akioni commented Sep 2, 2021

我总觉得你的 git commit 有点不太对劲,为啥每次都这么多 merge

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.

3 participants