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

fix(Ellipsis): Handle some post-update issues #6797

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix(Ellipsis): Handle some post-update issues #6797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f0a640
fix(Ellipsis): 非整数行高时，省略行数不符合预期 (#6792)
Dec 13, 2024
10cceed
fix(Ellipsis): walkingIndexes初始化覆盖计算结果,导致文本省略字数不符合预期 (#6792)
Dec 13, 2024
13b2c06
test(Ellipsis): 单测补充 (#6792)
Dec 13, 2024
b949787
fix(Ellipsis): 调整setWalkingIndexes调用时机
Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion src/components/ellipsis/tests/ellipsis.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ describe('Ellipsis', () => {
const that = this as HTMLElement
const charLen = (that.textContent || '').length || 1
const rows = Math.ceil(charLen / 30)
return rows * lineHeight
const styleLineHeight = parseFloat(that.style.lineHeight)
return Math.round(rows * (styleLineHeight || lineHeight))
},
},
})
Expand Down Expand Up @@ -132,4 +133,30 @@ describe('Ellipsis', () => {
fireEvent.click(getByText('expand'))
expect(getByText('collapse')).toBeInTheDocument()
})

test('non-integer line height', () => {
const rows = 2
const lineHeight = '16.4px'

const { getByTestId } = render(
<React.Fragment>
<Ellipsis
rows={rows}
style={{ lineHeight }}
content={content}
data-testid='ellipsis'
/>
<div style={{ lineHeight }} data-testid='maxheight'>
{content}
</div>
</React.Fragment>
)

const { offsetHeight } = getByTestId('ellipsis') || {}
const { offsetHeight: maxheight } = getByTestId('maxheight') || {}
const rowsHeight = Math.round(parseFloat(lineHeight) * rows)
const expectHeight = Math.min(rowsHeight, maxheight)

expect(offsetHeight).toBe(expectHeight)
})
})
4 changes: 2 additions & 2 deletions src/components/ellipsis/useMeasure.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ export default function useMeasure(
const midMeasureRef = React.useRef<HTMLDivElement>(null)

const startMeasure = useEvent(() => {
setStatus(MEASURE_STATUS.PREPARE)
setWalkingIndexes([
0,
direction === 'middle'
? Math.ceil(contentChars.length / 2)
: contentChars.length,
])
setStatus(MEASURE_STATUS.PREPARE)
})

// Initialize
Expand All @@ -66,7 +66,7 @@ export default function useMeasure(
const fullMeasureHeight = fullMeasureRef.current?.offsetHeight || 0
const singleRowMeasureHeight =
singleRowMeasureRef.current?.offsetHeight || 0
const rowMeasureHeight = singleRowMeasureHeight * rows
const rowMeasureHeight = singleRowMeasureHeight * (rows + 0.5)
Copy link
Member

Choose a reason for hiding this comment

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

能直接 +0.5 而不改 setWalkingIndexes 不，理论上启动阶段就是应该和 startMeasure 同步的

Copy link
Author

Choose a reason for hiding this comment

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

理论上是这样同步更新值，但是实际运行时，在线demo运行的结果却是不符合预期的。

从代码执行层面来说，其实只有状态为MEASURE_WALKING时才需要walkingIndexes的值，所以在设置MEASURE_WALKING状态前去更新walkingIndexes应该也是合理的。且这样做也能解决了结果不符合预期的问题。

Copy link
Author

Choose a reason for hiding this comment

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

对于重复计算的问题，您觉得这个点目前是否需要优化?
例如参考旧组件实现中缓存计算结果的做法，这样重新进入溢出判断时，不会一直命中计算条件。如果需要优化，我可以在这个迭代中一起处理。

Copy link
Member

Choose a reason for hiding this comment

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

理论上是这样同步更新值，但是实际运行时，在线demo运行的结果却是不符合预期的。

那直接包一个 unstable_batchedUpdates 告知其同时启动就行了

Copy link
Author

Choose a reason for hiding this comment

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

这个库之前没有使用这个函数，感觉还是不引入了，如果不移动，那放setState(PREPARE)前面也能解决，确保indexes更新在state更新前面

Copy link
Member

Choose a reason for hiding this comment

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

这个应该没关系，React 和 ReactDOM 总是配套出来的。可以用的


if (fullMeasureHeight <= rowMeasureHeight) {
setStatus(MEASURE_STATUS.STABLE_NO_ELLIPSIS)
Expand Down
Loading