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(AnimatingNumbers): 多端适配 #2700

Merged
merged 12 commits into from
Nov 21, 2024
2 changes: 1 addition & 1 deletion src/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,7 @@
"author": "swag~jun"
},
{
"version": "2.0.0",
"version": "3.0.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

3.0 标识涵盖鸿蒙适配,这种单独的 v14 适配,和 3.0 标识冲突。需要大家进行方案确认。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

3.0 标识涵盖鸿蒙适配,这种单独的 v14 适配,和 3.0 标识冲突。需要大家进行方案确认。

已确认

"name": "AnimatingNumbers",
"type": "component",
"cName": "数字动画",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,12 @@ test('test value props', () => {
const { container } = render(<AnimatingNumbers.CountUp value="678.94" />)

const listNumbers = container.querySelectorAll('.nut-countup-number')
expect(listNumbers[0]).toHaveAttribute(
'style',
'transition: transform 1s ease-in-out;'
)
expect(listNumbers[0]).toHaveAttribute('style', 'transition-duration: 1s;')
const defaultDelay = 300
vi.advanceTimersByTime(defaultDelay)
expect(listNumbers[0]).toHaveAttribute(
'style',
'transition: transform 1s ease-in-out; transform: translate(0, -30%);'
'transition-duration: 1s; transform: translate(0, -30%);'
)
expect(listNumbers.length).toBe(5)
})
Expand All @@ -32,7 +29,7 @@ test('test aysnc value and duration props', async () => {
vi.advanceTimersByTime(defaultDelay)
expect(listNumbers[0]).toHaveAttribute(
'style',
'transition: transform 1.2s ease-in-out; transform: translate(0, -50%);'
'transition-duration: 1.2s; transform: translate(0, -50%);'
)

for (let i = 0; i < 5; i++) {
Expand All @@ -45,7 +42,7 @@ test('test aysnc value and duration props', async () => {
const lastlen = value.length - 1
const lastNumber = Number(value.slice(lastlen))
const percentage = lastNumber === 0 ? 50 : 5 * lastNumber
const style = `transition: transform 0s ease-in-out; transform: translate(${0}, -${percentage}%);`
const style = `transition-duration: 0s; transform: translate(${0}, -${percentage}%);`
expect(listNumbers2[7]).toHaveAttribute('style', style)
}
})
41 changes: 41 additions & 0 deletions src/packages/animatingnumbers/animatingnumbers.harmony.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
.nut-countup-list {
display: inline-flex;
height: 32px;
overflow: hidden;
direction: ltr;
}
.nut-countup-listitem {
height: 32px;
overflow: hidden;
user-select: none;
-webkit-tap-highlight-color: transparent;
}
.nut-countup-listitem-number {
margin: 0 0;
border-radius: 4px;
color: #1a1a1a;
background-color: inherit;
}
.nut-countup-separator {
display: flex;
height: 80%;
align-items: flex-end;
color: inherit;
font-size: 18px;
font-weight: 600;
}
.nut-countup-number {
display: flex;
flex-direction: column;
align-items: center;
width: auto;
transition: transform 1s ease-in-out;
transform: translate(0, 0);
}
Comment on lines +27 to +34
Copy link

@coderabbitai coderabbitai bot Nov 5, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

优化动画性能

当前的动画实现可能在某些设备上出现性能问题。建议添加硬件加速和优化动画属性:

.nut-countup-number {
  display: flex;
  flex-direction: column;
  align-items: center;
  width: auto;
  transition: transform 1s ease-in-out;
  transform: translate(0, 0);
+ will-change: transform;
+ -webkit-transform: translate3d(0, 0, 0);
+ backface-visibility: hidden;
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.nut-countup-number {
display: flex;
flex-direction: column;
align-items: center;
width: auto;
transition: transform 1s ease-in-out;
transform: translate(0, 0);
}
.nut-countup-number {
display: flex;
flex-direction: column;
align-items: center;
width: auto;
transition: transform 1s ease-in-out;
transform: translate(0, 0);
will-change: transform;
-webkit-transform: translate3d(0, 0, 0);
backface-visibility: hidden;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

will-change 有可能存在布局影响,慎重考虑

Copy link

Choose a reason for hiding this comment

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

@oasis-cloud 感谢您的提醒,的确,使用 will-change 可能会对布局产生影响,我们会谨慎考虑这个建议。


✏️ Learnings added
Learnt from: oasis-cloud
PR: jdf2e/nutui-react#2700
File: src/packages/animatingnumbers/animatingnumbers.harmony.css:25-32
Timestamp: 2024-11-06T05:56:06.647Z
Learning: 在优化 NutUI React 动画性能时,添加 `will-change` 属性可能会对布局产生影响,需要谨慎使用。

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +27 to +34
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

优化动画性能

考虑到之前的经验,建议通过以下方式优化动画性能:

.nut-countup-number {
  display: flex;
  flex-direction: column;
  align-items: center;
  width: auto;
- transition: transform 1s ease-in-out;
+ transition: transform 1s cubic-bezier(0.23, 1, 0.32, 1);
  transform: translate(0, 0);
+ transform: translate3d(0, 0, 0);
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.nut-countup-number {
display: flex;
flex-direction: column;
align-items: center;
width: auto;
transition: transform 1s ease-in-out;
transform: translate(0, 0);
}
.nut-countup-number {
display: flex;
flex-direction: column;
align-items: center;
width: auto;
transition: transform 1s cubic-bezier(0.23, 1, 0.32, 1);
transform: translate3d(0, 0, 0);
}

.nut-countup-number-text {
height: 32px;
line-height: 32px;
color: #1a1a1a;
font-size: 18px;
font-weight: 600;
}
14 changes: 10 additions & 4 deletions src/packages/animatingnumbers/countup.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@
&-listitem {
height: $countup-height;
overflow: hidden;
color: $color-title;
font-size: $countup-base-size;
font-weight: $font-weight-bold;
user-select: none;
-webkit-tap-highlight-color: transparent;

&-number {
margin: 0 $countup-lr-margin;
Expand All @@ -26,17 +25,24 @@
height: 80%;
align-items: flex-end;
color: $countup-bg-color;
font-size: $countup-base-size;
font-weight: $font-weight-bold;
}

&-number {
display: flex;
flex-direction: column;
align-items: center;
width: $countup-width;
transition: transform 1s ease-in-out;
transform: translate(0, 0);

span {
&-text {
height: $countup-height;
line-height: $countup-height;
color: $countup-color;
font-size: $countup-base-size;
font-weight: $font-weight-bold;
}
}
}
90 changes: 49 additions & 41 deletions src/packages/animatingnumbers/countup.taro.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import React, {
useRef,
useState,
} from 'react'
import { View } from '@tarojs/components'
import { View, Text } from '@tarojs/components'
import { createSelectorQuery } from '@tarojs/taro'
import { BasicComponent, ComponentDefaults } from '@/utils/typings'
import { mergeProps } from '@/utils/merge-props'
Expand Down Expand Up @@ -39,7 +39,7 @@ export const CountUp: FunctionComponent<Partial<CountUpProps>> = (props) => {
} = mergeProps(defaultProps, props)
const classPrefix = 'nut-countup'
const countupRef = useRef<HTMLDivElement>(null)
const timerRef = useRef(0)
const timerRef = useRef()
const numbers = Array.from({ length: 10 }, (v, i) => i)

const getShowNumber = useCallback(() => {
Expand All @@ -54,65 +54,66 @@ export const CountUp: FunctionComponent<Partial<CountUpProps>> = (props) => {
return currNumber.split('')
}, [length, thousands, value])

const [numerArr, setNumerArr] = useState<string[]>([])
const [transformArr, setTransformArr] = useState<Array<string>>([])
const [numberArr, setNumberArr] = useState<string[]>([])
const [transformArr, setTransformArr] = useState<CSSProperties[]>([])
const isLoaded = useRef(false)

const setNumberTransform = useCallback(() => {
if (countupRef.current && numerArr.length) {
if (countupRef.current && numberArr.length) {
createSelectorQuery()
.selectAll('.nut-countup-listitem')
.node((numberItems: any) => {
const transformArrCache: string[] = []
const transformArrCache: CSSProperties[] = []
Object.keys(numberItems).forEach((key: any) => {
const elem = numberItems[Number(key)] as HTMLElement
const idx = Number(numerArr[Number(key)])
if (elem) {
const idx = Number(numberArr[Number(key)])
const enabled =
elem && typeof idx === 'number' && !Number.isNaN(idx)
if (enabled) {
// 计算规则:父元素和实际列表高度的百分比,分割成20等份
const transform =
idx || idx === 0
? `translate(0, -${(idx === 0 ? 10 : idx) * 5}%)`
: ''
transformArrCache.push(transform)
transformArrCache.push({
transitionDuration: `${duration}s`,
transform,
} as CSSProperties)
Comment on lines +78 to +81
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

优化了动画过渡效果的实现

使用对象形式管理样式属性是个很好的改进,但建议考虑性能优化:

-transformArrCache.push({
-  transitionDuration: `${duration}s`,
-  transform,
-} as CSSProperties)
+const style: CSSProperties = {
+  transitionDuration: `${duration}s`,
+  transform,
+  willChange: 'transform'
+}
+transformArrCache.push(style)

添加 willChange 属性可以提示浏览器提前优化动画性能。

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
transformArrCache.push({
transitionDuration: `${duration}s`,
transform,
} as CSSProperties)
const style: CSSProperties = {
transitionDuration: `${duration}s`,
transform,
willChange: 'transform'
}
transformArrCache.push(style)

}
})
setTransformArr([...transformArrCache])
})
.exec()
}
}, [numerArr])

const numberEaseStyle = (idx: number) => {
return {
transition: `transform ${duration}s ease-in-out`,
transform: transformArr[idx] ? transformArr[idx] : null,
} as CSSProperties
}

useEffect(() => {
setNumberTransform()
}, [numerArr, setNumberTransform])
}, [numberArr])

useEffect(() => {
if (!isLoaded.current) {
isLoaded.current = true
timerRef.current = window.setTimeout(() => {
setNumerArr(getShowNumber())
}, delay)
} else {
setNumerArr(getShowNumber())
if (numberArr.length) {
if (!isLoaded.current) {
isLoaded.current = true
timerRef.current = setTimeout(() => {
setNumberTransform()
}, delay)
} else {
setNumberTransform()
}
}
return () => {
window.clearTimeout(timerRef.current)
clearTimeout(timerRef.current)
isLoaded.current = false
}
}, [value, delay, getShowNumber])
}, [numberArr, delay, setNumberTransform])

useEffect(() => {
setNumberArr(getShowNumber())
}, [value, getShowNumber])

return (
<View className={`${classPrefix} ${className}`} ref={countupRef}>
<ul className={`${classPrefix}-list`}>
{numerArr.map((item: string, idx: number) => {
<View className={`${classPrefix} ${className}`} ref={countupRef} {...rest}>
<View className={`${classPrefix}-list`}>
{numberArr.map((item: string, idx: number) => {
return (
<li
<View
className={`${classPrefix}-listitem ${
!Number.isNaN(Number(item))
? `${classPrefix}-listitem-number`
Expand All @@ -121,21 +122,28 @@ export const CountUp: FunctionComponent<Partial<CountUpProps>> = (props) => {
key={idx}
>
{!Number.isNaN(Number(item)) ? (
<span
<View
className={`${classPrefix}-number`}
style={numberEaseStyle(idx)}
style={transformArr?.[idx]}
>
{[...numbers, ...numbers].map((number, subidx) => {
return <span key={subidx}>{number}</span>
return (
<Text
className={`${classPrefix}-number-text`}
key={subidx}
>
{number}
</Text>
)
})}
</span>
</View>
Comment on lines +125 to +139
Copy link

@coderabbitai coderabbitai bot Nov 5, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

渲染逻辑可以进行性能优化

当前实现在渲染数字时可能会导致不必要的重渲染。建议以下优化:

  1. 将数字渲染逻辑抽离为独立组件
  2. 使用 memo 优化渲染性能
+const NumberItem = memo(({ number }: { number: number }) => (
+  <Text className="nut-countup-number-text">
+    {number}
+  </Text>
+))
+
+NumberItem.displayName = 'NumberItem'

// 在原渲染逻辑中使用
-{[...numbers, ...numbers].map((number, subidx) => (
-  <Text className={`${classPrefix}-number-text`} key={subidx}>
-    {number}
-  </Text>
-))}
+{[...numbers, ...numbers].map((number, subidx) => (
+  <NumberItem key={subidx} number={number} />
+))}

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Collaborator

Choose a reason for hiding this comment

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

可以尝试

) : (
<span className={`${classPrefix}-separator`}>{item}</span>
<View className={`${classPrefix}-separator`}>{item}</View>
)}
</li>
</View>
)
})}
</ul>
</View>
</View>
)
}
Expand Down
21 changes: 14 additions & 7 deletions src/packages/animatingnumbers/countup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ export const CountUp: FunctionComponent<Partial<CountUpProps>> = (props) => {
return currNumber.split('')
}

const numerArr = useMemo(getShowNumber, [value, length, thousands])
const numberArr = useMemo(getShowNumber, [value, length, thousands])

const setNumberTransform = useCallback(() => {
if (countupRef.current) {
const numberItems = countupRef.current.querySelectorAll(
'.nut-countup-number'
)
const numberFilterArr: Array<string> = numerArr.filter(
const numberFilterArr: Array<string> = numberArr.filter(
(item: string) => !Number.isNaN(Number(item))
)
Object.keys(numberItems).forEach((key) => {
Expand All @@ -73,10 +73,10 @@ export const CountUp: FunctionComponent<Partial<CountUpProps>> = (props) => {
}
})
}
}, [numerArr])
}, [numberArr])

const numberEaseStyle: CSSProperties = {
transition: `transform ${duration}s ease-in-out`,
transitionDuration: `${duration}s`,
}

useEffect(() => {
Expand All @@ -86,12 +86,12 @@ export const CountUp: FunctionComponent<Partial<CountUpProps>> = (props) => {
return () => {
window.clearTimeout(timerRef.current)
}
}, [numerArr, delay, setNumberTransform])
}, [numberArr, delay, setNumberTransform])

return (
<div className={`${classPrefix} ${className}`} ref={countupRef}>
<ul className={`${classPrefix}-list`}>
{numerArr.map((item: string, idx: number) => {
{numberArr.map((item: string, idx: number) => {
return (
<li
className={`${classPrefix}-listitem ${
Expand All @@ -107,7 +107,14 @@ export const CountUp: FunctionComponent<Partial<CountUpProps>> = (props) => {
style={numberEaseStyle}
>
{[...numbers, ...numbers].map((number, subidx) => {
return <span key={subidx}>{number}</span>
return (
<span
className={`${classPrefix}-number-text`}
key={subidx}
>
{number}
</span>
)
})}
</span>
) : (
Expand Down
Loading