fix: 链接字段高亮下划线过长 #1652

lcx-seima · 2022-08-08T02:07:04Z

👀 PR includes

🎨 Enhance

🐛 Bugfix

Solve the issue and close #0

📝 Description

背景

当 spreadsheet container 被应用了特殊的文字样式后（如 letter-spacing）
[email protected] 绘制 链接下划线 会超过文字实际长度

原因

drawLinkFieldShape 绘制下划线时，使用的是 g-base (textShape.getBBox()) 计算出的文字宽度
而 g-base 使用的是离屏 canvas，不会应用 html 中任何样式，当计算文字尺寸时，与实际渲染有较大差距（如宽度）

之前在修复列头文字&icon 时，也发现了类似的问题。
当时把 s2 内部计算文字宽度的 offscreen canvas 放置到了 body，但实际场景中，特殊的文字样式是会设置到 container 这层，仅 body 的样式是不够的。

方案

直接使用 spreadsheet container 挂载的 canvas 作为文字宽度计算的 ctx：

废弃 utils/text.ts 中的 measureTextWidth、measureTextWidthRoughly 函数使用（有 export 暂不移除源码）
移动 measureTextWidth、measureTextWidthRoughly 逻辑，作为 spreadsheet 的实例方法
getEllipsisText 等文字工具函数，接受宽度计算函数 measureTextWidth 作为参数
drawLinkFieldShape 使用 this.actualTextWidth（由 s2 计算）作为超链接绘制宽度

🖼️ Screenshot

playground #root 设置了样式 letter-spacing: -1px;

Before	After

🔗 Related issue link

🔍 Self-Check before the merge

Add or update relevant docs.
Add or update relevant demos.
Add or update test case.
Add or update relevant TypeScript definitions.

vercel · 2022-08-08T02:07:06Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
antvis-s2	✅ Ready (Inspect)	Visit Preview	Aug 9, 2022 at 3:07AM (UTC)
s2	❌ Failed (Inspect)		Aug 9, 2022 at 3:07AM (UTC)

lgtm-com · 2022-08-08T02:13:21Z

This pull request introduces 1 alert and fixes 3 when merging e14bbf7 into 71fefc7 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

fixed alerts:

3 for Unused variable, import, function or class

github-actions · 2022-08-08T02:14:25Z

Size Change: +354 B (0%)

Total Size: 426 kB

Filename	Size	Change
`./packages/s2-core/dist/index.min.js`	165 kB	+354 B (0%)

ℹ️ View Unchanged

Filename	Size
`./packages/s2-core/dist/style.min.css`	389 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/@testing-library/dom.cjs.js`	21.7 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/@testing-library/dom.esm.js`	21.2 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/@testing-library/dom.umd.js`	60.3 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/@testing-library/dom.umd.min.js`	33.9 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/config.js`	1.16 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/DOMElementFilter.js`	1.87 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/event-map.js`	1.18 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/events.js`	1.55 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/get-node-text.js`	300 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/get-queries-for-element.js`	738 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/get-user-code-frame.js`	810 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/helpers.js`	1.07 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/index.js`	1.14 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/label-helpers.js`	1.05 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/matches.js`	1.14 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/pretty-dom.js`	1.47 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/queries/all-utils.js`	337 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/queries/alt-text.js`	573 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/queries/display-value.js`	713 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/queries/index.js`	418 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/queries/label-text.js`	1.8 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/queries/placeholder-text.js`	491 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/queries/role.js`	2.67 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/queries/test-id.js`	498 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/queries/text.js`	958 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/queries/title.js`	718 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/query-helpers.js`	1.81 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/role-helpers.js`	2.82 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/screen.js`	1.51 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/suggestions.js`	1.5 kB
`./packages/s2-core/node_modules/@testing-library/dom/dist/wait-for-element-to-be-removed.js`	748 B
`./packages/s2-core/node_modules/@testing-library/dom/dist/wait-for.js`	2.38 kB
`./packages/s2-react/dist/index.min.js`	64.1 kB
`./packages/s2-react/dist/style.min.css`	3.21 kB
`./packages/s2-vue/dist/index.min.js`	20.4 kB
`./packages/s2-vue/dist/style.min.css`	1.61 kB

_{compressed-size-action}

packages/s2-core/src/sheet-type/spread-sheet.ts

github-actions · 2022-08-08T02:29:21Z

你好, @lcx-seima CI 执行失败, 请点击 [Details] 按钮查看, 并根据日志修复。

Hello, @lcx-seima CI run failed, please click the [Details] button for detailed log information and fix it.

packages/s2-core/src/cell/corner-cell.ts

packages/s2-core/src/sheet-type/spread-sheet.ts

lgtm-com · 2022-08-08T06:33:23Z

This pull request introduces 1 alert and fixes 3 when merging 3627212 into 71fefc7 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

fixed alerts:

3 for Unused variable, import, function or class

codecov · 2022-08-08T06:55:46Z

Codecov Report

Merging #1652 (c89d41e) into master (71fefc7) will decrease coverage by 0.17%.
The diff coverage is 73.91%.

@@            Coverage Diff             @@
##           master    #1652      +/-   ##
==========================================
- Coverage   77.17%   77.00%   -0.18%     
==========================================
  Files         231      231              
  Lines       11177    11194      +17     
  Branches     2355     2360       +5     
==========================================
- Hits         8626     8620       -6     
- Misses       1236     1257      +21     
- Partials     1315     1317       +2

Impacted Files	Coverage Δ
packages/s2-core/src/cell/base-cell.ts	`87.80% <ø> (ø)`
packages/s2-core/src/facet/header/series-number.ts	`100.00% <ø> (ø)`
packages/s2-core/src/utils/g-mini-charts.ts	`52.10% <0.00%> (ø)`
packages/s2-core/src/utils/text.ts	`33.67% <33.33%> (-6.74%)`	⬇️
packages/s2-core/src/facet/table-facet.ts	`85.89% <90.00%> (-0.04%)`	⬇️
packages/s2-core/src/cell/corner-cell.ts	`77.14% <100.00%> (+0.21%)`	⬆️
packages/s2-core/src/facet/pivot-facet.ts	`82.14% <100.00%> (ø)`
packages/s2-core/src/sheet-type/spread-sheet.ts	`80.72% <100.00%> (-0.89%)`	⬇️
packages/s2-core/src/utils/canvas.ts	`50.00% <0.00%> (-37.50%)`	⬇️
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

lgtm-com · 2022-08-09T03:07:37Z

This pull request introduces 1 alert and fixes 3 when merging c89d41e into 577cd84 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

fixed alerts:

3 for Unused variable, import, function or class

* feat: 当前只能复制数值可带表头复制 (#1590) * feat: 添加通过 cellMetas 获取单元格对应的列头文本的方法 * feat: 添加通过 cellMetas 获取单元格对应的行头文本的方法 * feat: 只复制数据单元格时可以携带行列头 * feat: 选择某X行/列数据时可以携带行列头 * feat: 提取矩阵转换为字符串的方法 * test: 添加带行列头复制的单测 * test: 明细表添加带行列头复制的单测 * docs: 复制数据是否带表头信息 * feat(interaction): 行列宽高支持控制拖拽范围 (#1583) * feat(interaction): 行列宽高支持控制拖拽范围 * feat(interaction): 增加测试和文档 * feat(interaction): 增加主题色 * Update row-column-resize-spec.ts * Update packages/s2-core/src/interaction/row-column-resize.ts Co-authored-by: Wenjun Xu <[email protected]> * fix: rename Co-authored-by: Wenjun Xu <[email protected]> * docs: 复制数据是否带表头信息 * refactor: 评审细节修改 Co-authored-by: zishang <[email protected]> Co-authored-by: Jinke Li <[email protected]> Co-authored-by: Wenjun Xu <[email protected]> * fix(layout): 修复 Firefox 浏览器部分 icon 渲染失败 close #1571 (#1599) * fix: 有冻结行且有垂直scrollWidth时冻结行无法 resize (#1594) * fix: 有冻结行且有垂直scrollWidth时冻结航无法resize * fix: 有冻结行且有垂直scrollWidth时冻结航无法resize Co-authored-by: owen.wjh <[email protected]> * chore: 🤖 更新 lock 和 changelog 文件 (#1601) * fix: sortByFunc 排序后行列维度节点丢失 (#1606) * fix(interaction): 向左移动到不完全可见cell的时候，没有滚动过去 (#1607) * fix: 明细表复制时无需使用formatter格式化列头label (#1610) * fix: 明细表复制时无需使用formatter格式化列头label * test: 完善明细表导出case * feat: 支持resize最右侧column (#1611) * feat: 支持resize最右侧column * fix: 添加testing-library/dom dep * fix: npm源 * fix: npm源 * fix: 切换成dispatchEvent * fix: testcase fix Co-authored-by: owen.wjh <[email protected]> * docs: 将 S2Event 提取到 API 文档中, 方便查看 (#1618) * fix(strategysheet): 修复趋势分析表列头格式化不生效 (#1616) * fix(strategysheet): 修复趋势分析表列头格式化不生效 * fix: update * test: 增加测试 * fix: test * docs: 优化文档说法和校正有误的内容 (#1640) * docs: 让 demo 展示符合主题 * docs: 排序文档优化 * docs: 合并单元格文档更新 * docs: getAllCells()获取数据说明问题 * fix: 修复存在 0 时，数值为 number 时，排序错误的问题 (#1644) * fix: 修复存在 0 时，排序错误的问题 * test: 补充存在 0 时，排序错误的单测 * fix: 修复单测字典序排序错误 Co-authored-by: zishang <[email protected]> * fix(pagination): 分页配置未传 current 参数时表格渲染空白 (#1633) * feat: 复制支持html格式 (#1647) * feat: 复制支持html格式 * fix: 修复复制测试用例 * fix: 添加复制格式测试用例 * Update packages/s2-core/src/utils/export/copy.ts Co-authored-by: stone <[email protected]> * fix: moved mimetype as enum * chore: generic type narrowing * fix: html escaping special chars Co-authored-by: owen.wjh <[email protected]> Co-authored-by: stone <[email protected]> * feat(interaction): 宽高调整事件透出 resizedWidth/resizedHeight, 修复错误类型定义 (#1638) * feat(interaction): 宽高调整事件透出 resizedWidth/resizedHeight, 修复错误类型定义 * docs: 优化文档 * chore: 更新 reviewer * fix: 修复pivot模式 cell点击事件无法触发BUG (#1623) fix: 修复pivot-table cell事件无法触发BUG * chore: 发布成功后自动部署官网 (#1649) * fix(layout): 修复 treeRowsWidth 配置不生效 close #1622 (#1646) * fix(layout): 修复 treeRowsWidth 配置不生效 close #1622 * test: 修复测试 * chore: 🤖 更新 lock 和 changelog 文件 (#1650) * fix: 修复选中态的描边宽度样式问题 (#1654) * fix: 链接字段高亮下划线过长 (#1652) * refactor: measureTextWidth移动为spreadsheet实例方法 * test: 调整 text 测试用例 * feat: 明细表行头单元格支持拖拽 (#1655) * feat: 明细表行头单元格支持拖拽 * feat: 明细表行头单元格支持拖拽 * test: 添加单测 * fix(interaction): 优化resetSheetStyle性能 (#1653) * fix: 优化reset性能 * fix: icon draw * docs: 官网文档优化 (#1661) * fix(scroll): 修复滚动边界判断错误导致无法滚动 (#1664) Co-authored-by: 卿珂 <[email protected]> * chore: update the reviewers (#1673) * fix(resize): 修复总计列存在子节点时无法调整宽度 (#1675) * fix: 修复明细表 onRaneSort 失效问题 (#1678) * docs: 修改排序API文档 (#1677) * refactor: 重构多指标绘制文本布局逻辑 * feat: 趋势分析表支持icon字段标记 Co-authored-by: stone <[email protected]> Co-authored-by: zishang <[email protected]> Co-authored-by: Jinke Li <[email protected]> Co-authored-by: Wenjun Xu <[email protected]> Co-authored-by: serializedowen <[email protected]> Co-authored-by: owen.wjh <[email protected]> Co-authored-by: 刘嘉一 <[email protected]> Co-authored-by: YardWill <[email protected]> Co-authored-by: xiaochong44 <[email protected]> Co-authored-by: 卿珂 <[email protected]>

lijinke666 · 2022-08-12T10:16:44Z

🎉 This PR is included in version @antv/s2-v1.26.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

lijinke666 · 2022-08-15T12:13:49Z

🎉 This PR is included in version @antv/s2-v1.26.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

lijinke666 · 2022-11-25T07:58:05Z

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

* refactor: measureTextWidth移动为spreadsheet实例方法 * test: 调整 text 测试用例

lcx-seima added 2 commits August 7, 2022 23:41

refactor: measureTextWidth移动为spreadsheet实例方法

e63f699

test: 调整 text 测试用例

e14bbf7

github-actions bot assigned lcx-seima Aug 8, 2022

github-actions bot requested review from GaoFuhong, lijinke666, qubaomingg, stone-lyl, wjgogogo, xingwanying and YardWill August 8, 2022 02:07

vercel bot had a problem deploying to Preview – s2 August 8, 2022 02:14 Failure

vercel bot deployed to Preview – antvis-s2 August 8, 2022 02:18 View deployment

wjgogogo reviewed Aug 8, 2022

View reviewed changes

packages/s2-core/src/sheet-type/spread-sheet.ts Outdated Show resolved Hide resolved

github-actions bot added the 🚨 test failed 单元测试挂了 label Aug 8, 2022

lcx-seima changed the title ~~[WIP] fix: 链接字段高亮下划线过长~~ fix: 链接字段高亮下划线过长 Aug 8, 2022

github-actions bot added the pr(fix) bug fix label Aug 8, 2022

test: 修复table-facet用例

02e159b

xingwanying reviewed Aug 8, 2022

View reviewed changes

packages/s2-core/src/cell/corner-cell.ts Show resolved Hide resolved

packages/s2-core/src/sheet-type/spread-sheet.ts Show resolved Hide resolved

xingwanying enabled auto-merge (squash) August 8, 2022 03:58

lcx-seima added 2 commits August 8, 2022 14:23

test: 修复 pivot facet 树状测试用例

75684ed

fix: 优化参数字符串化

3627212

vercel bot deployed to Preview August 8, 2022 06:37 View deployment

github-actions bot removed the 🚨 test failed 单元测试挂了 label Aug 8, 2022

lcx-seima requested a review from xingwanying August 8, 2022 07:26

lcx-seima requested a review from wjgogogo August 8, 2022 07:26

chore: deprecated 注释调整

c89d41e

vercel bot deployed to Preview August 9, 2022 03:07 View deployment

xingwanying approved these changes Aug 9, 2022

View reviewed changes

xingwanying merged commit 4a79470 into master Aug 9, 2022

xingwanying deleted the fix/measure-text-91 branch August 9, 2022 03:37

lijinke666 added the released on @alpha label Aug 12, 2022

lijinke666 added the released label Aug 15, 2022

wjgogogo pushed a commit that referenced this pull request Oct 25, 2023

fix: 链接字段高亮下划线过长 (#1652)

9c39717

* refactor: measureTextWidth移动为spreadsheet实例方法 * test: 调整 text 测试用例

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: 链接字段高亮下划线过长 #1652

fix: 链接字段高亮下划线过长 #1652

lcx-seima commented Aug 8, 2022 •

edited

Loading

vercel bot commented Aug 8, 2022 •

edited

Loading

lgtm-com bot commented Aug 8, 2022

github-actions bot commented Aug 8, 2022 •

edited

Loading

github-actions bot commented Aug 8, 2022

lgtm-com bot commented Aug 8, 2022

codecov bot commented Aug 8, 2022 •

edited

Loading

lgtm-com bot commented Aug 9, 2022

lijinke666 commented Aug 12, 2022

lijinke666 commented Aug 15, 2022

lijinke666 commented Nov 25, 2022

fix: 链接字段高亮下划线过长 #1652

fix: 链接字段高亮下划线过长 #1652

Conversation

lcx-seima commented Aug 8, 2022 • edited Loading

👀 PR includes

📝 Description

背景

原因

方案

🖼️ Screenshot

🔗 Related issue link

🔍 Self-Check before the merge

vercel bot commented Aug 8, 2022 • edited Loading

lgtm-com bot commented Aug 8, 2022

github-actions bot commented Aug 8, 2022 • edited Loading

github-actions bot commented Aug 8, 2022

lgtm-com bot commented Aug 8, 2022

codecov bot commented Aug 8, 2022 • edited Loading

Codecov Report

lgtm-com bot commented Aug 9, 2022

lijinke666 commented Aug 12, 2022

lijinke666 commented Aug 15, 2022

lijinke666 commented Nov 25, 2022

lcx-seima commented Aug 8, 2022 •

edited

Loading

vercel bot commented Aug 8, 2022 •

edited

Loading

github-actions bot commented Aug 8, 2022 •

edited

Loading

codecov bot commented Aug 8, 2022 •

edited

Loading