-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
refactor(Carousel): refactor Carousel
#2061
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/tusimple/naive-ui/AXvmaZM3PGLJ9hJrRXMrzFRD21aA |
This pull request introduces 1 alert when merging 7ca2a6b into 2870228 - view on LGTM.com new alerts:
|
更新一下测试 |
😉想一块去了 |
Codecov Report
@@ Coverage Diff @@
## main #2061 +/- ##
==========================================
+ Coverage 64.51% 64.64% +0.13%
==========================================
Files 898 905 +7
Lines 17622 18129 +507
Branches 4176 4326 +150
==========================================
+ Hits 11368 11719 +351
- Misses 5464 5588 +124
- Partials 790 822 +32
Continue to review full report at Codecov.
|
2021-12-31.12.47.32.mov每次 refresh 的时候都会默认播放一下,需要处理 |
是有这个问题,我处理一下 |
2022-01-05.2.26.47.mov2022-01-05.2.27.32.movTransition 还是有问题 你应该是需要使用 isMounted 来判断一下应不应该 apply transtion naive 很多地地方都会针对性的在 mounted 的时候关掉 transition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import { useConfig, useTheme } from '../../_mixins' | ||
import type { ThemeProps } from '../../_mixins' | ||
import { on, off } from 'evtd' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
package import 都放在最前面
src/carousel/src/Carousel.tsx
Outdated
isTouchEvent, | ||
getRealityIndex | ||
} from './utils' | ||
import { VResizeObserver } from 'vueuc' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
package import 都放在最前面
src/carousel/src/Carousel.tsx
Outdated
const nextRealityIndex = getNextRealityIndex(index) | ||
return nextRealityIndex !== null && nextRealityIndex === index | ||
} | ||
function isDisabledPrev (): boolean { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isPrevDisabled
src/carousel/src/CarouselArrow.tsx
Outdated
{ | ||
[`${mergedClsPrefix}-carousel__arrow--disabled`]: this.isDisabledPrev() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.isDisabledPrev() && ${mergedClsPrefix}-carousel__arrow--disabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这么搞行数少,好看点
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2022-01-05.2.26.47.mov
2022-01-05.2.27.32.mov
Transition 还是有问题你应该是需要使用 isMounted 来判断一下应不应该 apply transtion
naive 很多地地方都会针对性的在 mounted 的时候关掉 transition
好,这个应该是监听size的时候动的。不得不说你的电脑刷新好快,我都复现不出这种效果。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
M1 还是牛逼的 😂
了解,内部其实做了切换的缓冲处理,只是丢给用户的 method 上面没有处理
这个实例是自己自定义的,用的作用域插槽,所以部分事件没有内部的处理,内部的点可以focus |
没影响到,我们只对拖拽时的点击事件进行了阻止 |
OK |
Card 切换模式的感觉有点问题。 2022-01-09.5.21.45.mov2022-01-09.5.22.22.mov |
Card 模式还有一个问题,从第一个会直接切换到第三个 2022-01-09.5.23.37.mov |
这个 DEMO 在不同模式切换的效果不好,给 carousel 来个 key 吧,强行切换 DOM
2022-01-09.5.24.13.mov |
This pull request introduces 1 alert when merging 04264d5 into 71fa285 - view on LGTM.com new alerts:
|
closes #1931, #1462, #1159, #1892, #1866, #1882, #1777.