Skip to content

Conversation

json-q
Copy link
Owner

@json-q json-q commented Sep 6, 2025

Summary by CodeRabbit

  • 新功能
    • 新增 Tag 组件:支持 sm/md/lg 尺寸、边框、图标、自定义颜色、可关闭与自定义关闭图标,提供点击与关闭回调;支持全局配置;对外导出 Tag 及类型并加入索引导出。
  • 文档
    • 新增 Tag 组件文档与示例页(基础、尺寸、动态增删、自定义颜色)。
  • 样式
    • 新增样式变量、尺寸混入与组件样式,覆盖有色、边框、图标与交互态。
  • 测试
    • 增加单元测试,覆盖渲染、属性、事件回调与上下文尺寸继承。

Copy link

coderabbitai bot commented Sep 6, 2025

Walkthrough

在 sqi-web 包中新增 Tag 组件:实现组件、类型、样式、测试、文档与示例;在组件配置与包入口中加入 Tag 的对外导出与配置支持;样式包含变量与尺寸混入;测试覆盖渲染、尺寸、颜色、可关闭行为与上下文尺寸继承。

Changes

Cohort / File(s) Summary
ConfigProvider 类型扩展
packages/sqi-web/src/config-provider/type.ts
引入 TagProps 类型;在 ComponentConfig 中新增可选属性 Tag?: TagProps;修正 TooltipProps 的导入路径。
公共导出入口
packages/sqi-web/src/index.ts, packages/sqi-web/src/tag/index.ts
包级入口新增 export * from './tag';Tag 模块导出默认与命名导出 Tag,并导出 TagProps 类型。
Tag 组件实现与类型
packages/sqi-web/src/tag/Tag.tsx, packages/sqi-web/src/tag/type.ts
新增 Tag 组件(forwardRef span):支持尺寸、边框、图标、颜色、可关闭(含自定义关闭图标)与 onClose 阻止默认隐藏;定义 TagProps 类型。
样式与尺寸混入
packages/sqi-web/src/tag/style/_var.scss, .../style/_mixin.scss, .../style/index.scss, .../style/index.ts
新增样式变量(尺寸、颜色、图标间距等)、tag-size 混入;实现基础样式、边框/自定义颜色态、关闭图标样式与 sm/md/lg 尺寸;入口侧效导入样式。
测试
packages/sqi-web/src/tag/__tests__/tag.test.tsx
新增测试覆盖渲染、类名与样式、尺寸、title、边框、图标、颜色、点击、可关闭及自定义关闭图标、onClose 阻止默认、ConfigProvider 尺寸继承等。
文档与示例
packages/sqi-web/src/tag/index.md, packages/sqi-web/src/tag/demos/base.tsx, .../demos/size.tsx, .../demos/custom-color.tsx, .../demos/edit.tsx
新增文档页与示例:基础用法、尺寸展示、自定义颜色、可编辑增删示例。

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant CP as ConfigProvider
  participant T as Tag
  participant CI as CloseIcon

  U->>T: 渲染 <Tag ... />
  T->>CP: 读取上下文(prefixCls, size, componentConfig)
  CP-->>T: 提供配置(含默认 size)
  T-->>U: 展示标签(可选图标/颜色/边框/关闭)

  alt 用户点击 Tag
    U->>T: onClick(e)
    T-->>U: 触发 onClick 回调
  end

  opt closable
    U->>CI: 点击关闭图标
    CI->>T: onInnerClose(e)
    T->>T: 调用 onClose(e)
    alt e.defaultPrevented
      T-->>U: 保持可见
    else 未阻止默认
      T->>T: 设置 innerVisible=false
      T-->>U: 从 DOM 隐藏
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • feat: add tooltip #46: 同样改动了 ComponentConfig 与包入口导出(添加 Tooltip),与本次添加 Tag 的公共 API 变更相似。
  • feat: add trigger #29: 在同一文件扩展了 ComponentConfig(添加 Trigger),与本次对 packages/sqi-web/src/config-provider/type.ts 的变更直接相关。
  • feat: add switch component #34: 引入新组件并更新公共导出与配置(添加 Switch),与本 PR 的模式一致。

Poem

小兔写下一枚签,轻轻落在 UI 间。
左拥图标右随边,色彩随心不撒欢。
若点小叉悄然散,阻止默认仍相伴。
新枝入库随风转,Tag 已就位,请君看。 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/tag

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (19)
packages/sqi-web/src/config-provider/type.ts (1)

55-55: 在 ComponentConfig 中加入 Tag:LGTM

类型面向配置层的扩展正确;后续建议在文档的 API 区补充该配置项说明,便于用户通过 ConfigProvider 全局设置 Tag 默认属性。

packages/sqi-web/src/tag/index.md (1)

13-23: 建议补充 API 与全局配置说明

文档目前仅含示例,建议新增:

  • API:列出 TagProps(size、bordered、icon、closable、onClose、color、title 等)。
  • 全局配置:在 ConfigProvider > componentConfig 中 Tag 的可配项示例。

我可以基于现有组件风格补一版 API 表格与 ConfigProvider 示例。

packages/sqi-web/src/tag/demos/custom-color.tsx (2)

24-24: 使用稳定且语义化的 key 值

示例中 key 使用索引,建议改为使用颜色值,避免潜在重排时的 diff 异常(尽管本示例数组为静态)。

应用如下修改:

-        <Tag key={i.toString()} closable color={color}>
+        <Tag key={color} closable color={color}>

4-18: 常量数组可声明为只读字面量,便于类型收窄与避免意外修改

对示例无功能性影响,但更稳健。

可选改动:

-const COLORS_CUSTOM = [
+const COLORS_CUSTOM = [
   '#f53f3f',
   '#7816ff',
   '#00b42a',
   '#165dff',
   '#ff7d00',
   '#eb0aa4',
   '#7bc616',
   '#86909c',
   '#b71de8',
   '#0fc6c2',
   '#ffb400',
   '#168cff',
   '#ff5722',
-];
+ ] as const;
packages/sqi-web/src/tag/demos/base.tsx (1)

16-21: 文案与属性命名保持一致

建议将展示文案由“closeable”统一为“closable”,与组件属性命名一致;同时在“custom close icon”示例里也可加一个 onClose 展示可组合性(可选)。

-      <Tag closable>closeable</Tag>
+      <Tag closable>closable</Tag>
packages/sqi-web/src/tag/styles/_mixin.scss (1)

7-15: 为非法 size 做兜底并改进排版稳健性

  • 当传入未定义的 size 时当前会得到 null,建议显式报错,便于在构建期或调试期暴露问题。
  • 行高可与高度同步,减少文本裁剪风险(尤其非 flex 布局时)。
 @mixin tag-size($size) {
-  $map-size: map.get($tag-size, $size);
+  @if map.has-key($tag-size, $size) == false {
+    @error 'Invalid tag size: #{ $size }. Available: #{ map.keys($tag-size) }';
+  }
+  $map-size: map.get($tag-size, $size);
 
   &.#{$module}-size-#{$size} {
     font: map.get($map-size, 'font');
     height: map.get($map-size, 'height');
+    line-height: map.get($map-size, 'height');
     padding: map.get($map-size, 'padding');
   }
 }
packages/sqi-web/src/tag/demos/edit.tsx (3)

10-18: 避免直接修改 React 状态(引用不变导致潜在不更新)

用不可变更新,顺便去除空白值。

-  function addTag() {
-    if (inputValue) {
-      tags.push(inputValue);
-      setTags(tags);
-      setInputValue('');
-    }
-
-    setShowInput(false);
-  }
+  function addTag() {
+    const value = inputValue.trim();
+    if (value) {
+      setTags((prev) => [...prev, value]);
+      setInputValue('');
+    }
+    setShowInput(false);
+  }

27-33: 确保 key 稳定且唯一

当用户添加重复标签时,key={tag} 会冲突。可拼接下标或使用 uuid(示例用下标即可)。

-          <Tag key={tag} closable={index !== 0} onClose={() => removeTag(tag)}>
+          <Tag key={`${tag}-${index}`} closable={index !== 0} onClose={() => removeTag(tag)}>

45-47: 补充键盘可达性(Enter/Space 打开输入)

示例里已加 tabIndex,可顺手加 onKeyDown。

-        <Tag icon={<AddIcon />} className="add-tag" tabIndex={0} onClick={() => setShowInput(true)}>
+        <Tag
+          icon={<AddIcon />}
+          className="add-tag"
+          tabIndex={0}
+          onClick={() => setShowInput(true)}
+          onKeyDown={(e) => (e.key === 'Enter' || e.key === ' ') && setShowInput(true)}
+        >
packages/sqi-web/src/tag/type.ts (1)

7-9: 去除与 HTMLAttributes 重复的字段,减少遮蔽与维护成本

className/style/title/onClick 已包含于 HTMLAttributes<HTMLSpanElement>,无需重复声明。

-  className?: string;
-  style?: CSSProperties;
+  // className/style/title/onClick 已包含于 HTMLAttributes
@@
-  title?: string;
@@
-  onClick?: (e: MouseEvent<HTMLElement>) => void;

Also applies to: 11-11, 17-17

packages/sqi-web/src/tag/styles/index.scss (4)

1-1: 移除未使用的 sass:map 导入

本文件未使用 sass:map,可移除以避免无谓依赖与编译负担。

-@use 'sass:map';

11-17: 建议补充 box-sizing 以保证尺寸可预期

为避免内外边距/边框影响布局,可在根样式处加上 box-sizing: border-box;

   display: inline-flex;
   align-items: center;
+  box-sizing: border-box;
   border-radius: $border-radius-md;

38-42: 仅对带有 “-icon” 类名的图标生效,建议在文档或代码中明确

此选择器只会作用于拥有 ${prefix}-icon 的图标(由 Icon 组件产出)。若用户传入任意自定义节点(如纯 <span>),不会应用到间距与尺寸。可在文档标注或在组件层为 icon 自动包一层 Icon。


44-56: 为关闭图标增加键盘可访问态与焦点可视样式

当前仅 hover 变色,建议补充 :focus-visible,提升无鼠标场景的可达性。

   & &-close-icon {
     margin-right: 0;
     margin-left: $tag-close-icon-ml;
     color: $tag-close-icon-color;
     font: $font-md;
     cursor: pointer;
     transition: color $anim-duration-base $anim-time-fn-easing;
 
+    &:focus-visible {
+      outline: 2px solid $tag-close-icon-hover-color;
+      outline-offset: 2px;
+    }
+
     &:hover {
       color: $tag-close-icon-hover-color;
     }
   }
packages/sqi-web/src/tag/__tests__/tag.test.tsx (2)

95-101: 补充校验:自定义 close 图标应保留自身 className/onClick

当前组件实现会覆盖自定义 close 图标的 classNameonClick。修复后建议在此用例里也断言合并后的类名以及自定义 onClick 能被调用一次(且未 preventDefault 时仍会触发关闭)。

如需,我可以一并补充对应测试断言。


71-79: 轻量建议:使用 toHaveBeenCalledTimes 语义更清晰

toHaveBeenCalledTimes 更贴合断言语义(Vitest/Jest 等价)。

-    expect(handleClick).toBeCalledTimes(1);
+    expect(handleClick).toHaveBeenCalledTimes(1);
packages/sqi-web/src/tag/Tag.tsx (2)

9-9: defaultProps 类型过宽且当前为空,建议收窄为 Partial

没有默认项时可直接移除或使用 Partial<TagProps>,减少无意义的类型扩张。

-const defaultProps: TagProps = {};
+const defaultProps: Partial<TagProps> = {};

68-76: 可点击的非交互元素缺少语义与键盘支持(可选)

当存在 onClick 时可为外层 <span> 增加 role="button"tabIndex=0,并处理 Enter/Space 键,增强可达性(已对关闭图标处理,外层可按需补充)。

packages/sqi-web/src/tag/styles/_var.scss (1)

14-18: 尺寸 token 合理性核对(可选)

lg 的水平内边距为 $padding-xs,小于 md$padding-sm。若非设计有意,建议与设计核对是否应为更大的 padding。

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc32ada and 52949c2.

⛔ Files ignored due to path filters (1)
  • packages/sqi-web/src/tag/__tests__/__snapshots__/tag.test.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (14)
  • packages/sqi-web/src/config-provider/type.ts (2 hunks)
  • packages/sqi-web/src/index.ts (1 hunks)
  • packages/sqi-web/src/tag/Tag.tsx (1 hunks)
  • packages/sqi-web/src/tag/__tests__/tag.test.tsx (1 hunks)
  • packages/sqi-web/src/tag/demos/base.tsx (1 hunks)
  • packages/sqi-web/src/tag/demos/custom-color.tsx (1 hunks)
  • packages/sqi-web/src/tag/demos/edit.tsx (1 hunks)
  • packages/sqi-web/src/tag/index.md (1 hunks)
  • packages/sqi-web/src/tag/index.ts (1 hunks)
  • packages/sqi-web/src/tag/styles/_mixin.scss (1 hunks)
  • packages/sqi-web/src/tag/styles/_var.scss (1 hunks)
  • packages/sqi-web/src/tag/styles/index.scss (1 hunks)
  • packages/sqi-web/src/tag/styles/index.ts (1 hunks)
  • packages/sqi-web/src/tag/type.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
packages/sqi-web/src/tag/index.ts (1)
packages/sqi-web/gulpfile.js (1)
  • file (93-100)
packages/sqi-web/src/tag/demos/custom-color.tsx (5)
packages/sqi-web/src/tag/demos/base.tsx (1)
  • Demo (5-23)
packages/sqi-web/src/space/index.ts (1)
  • Space (5-5)
packages/sqi-web/src/tag/index.ts (1)
  • Tag (5-5)
packages/sqi-web/src/space/demos/base.tsx (1)
  • Demo (4-33)
packages/sqi-web/src/space/demos/wrap.tsx (1)
  • Demo (4-14)
packages/sqi-web/src/tag/type.ts (1)
packages/sqi-web/src/config-provider/type.ts (1)
  • ConfigSize (16-16)
packages/sqi-web/src/tag/demos/base.tsx (4)
packages/sqi-web/src/tag/demos/custom-color.tsx (1)
  • Demo (20-30)
packages/sqi-web/src/tag/demos/edit.tsx (1)
  • Demo (5-51)
packages/sqi-web/src/space/index.ts (1)
  • Space (5-5)
packages/sqi-web/src/tag/index.ts (1)
  • Tag (5-5)
packages/sqi-web/src/index.ts (1)
packages/sqi-web/gulpfile.js (2)
  • file (93-100)
  • copyRenameStyleIndex (81-103)
packages/sqi-web/src/tag/styles/index.ts (1)
packages/sqi-web/gulpfile.js (7)
  • copyRenameStyleIndex (81-103)
  • cssInjection (70-75)
  • file (93-100)
  • generateCssJs (45-60)
  • mergeCssToDist (19-27)
  • compilerResetScss (30-43)
  • compilerScss (8-16)
packages/sqi-web/src/config-provider/type.ts (2)
packages/sqi-web/src/tag/type.ts (1)
  • TagProps (4-18)
packages/sqi-web/src/tooltip/type.ts (1)
  • TooltipProps (3-9)
packages/sqi-web/src/tag/__tests__/tag.test.tsx (1)
packages/sqi-web/src/tag/index.ts (1)
  • Tag (5-5)
packages/sqi-web/src/tag/Tag.tsx (5)
packages/sqi-web/src/tag/type.ts (1)
  • TagProps (4-18)
packages/sqi-web/src/config-provider/context.ts (1)
  • ConfigContext (10-10)
packages/sqi-hooks/src/useMergeProps.ts (1)
  • useMergeProps (3-22)
packages/sqi-icons/src/icons/CloseIcon.tsx (1)
  • CloseIcon (4-22)
packages/sqi-web/src/config-provider/ConfigProvider.tsx (1)
  • ConfigProvider (9-21)
packages/sqi-web/src/tag/demos/edit.tsx (4)
packages/sqi-web/src/space/index.ts (1)
  • Space (5-5)
packages/sqi-web/src/tag/index.ts (1)
  • Tag (5-5)
packages/sqi-web/src/input/index.ts (1)
  • Input (5-5)
packages/sqi-web/src/icon/demos/base.tsx (1)
  • Demo (5-15)
packages/sqi-web/src/tag/styles/_var.scss (1)
packages/sqi-icons/src/icons/CloseIcon.tsx (1)
  • CloseIcon (4-22)
packages/sqi-web/src/tag/styles/index.scss (1)
packages/sqi-web/gulpfile.js (1)
  • cssInjection (70-75)
🔇 Additional comments (8)
packages/sqi-web/src/index.ts (1)

12-12: 公开导出 Tag 模块:LGTM

与现有导出模式保持一致,无副作用导出问题。

packages/sqi-web/src/config-provider/type.ts (1)

12-13: 保留现状
已确认 packages/sqi-web/src/tag/index.tspackages/sqi-web/src/tooltip/index.ts 均正确 re-export 对应类型,且无循环依赖,可保持现状。

packages/sqi-web/src/tag/index.ts (2)

1-6: 导出设计清晰,API 直观

同时提供命名与默认导出,类型单独导出,符合当前项目的导出风格。


1-6: 确认构建链能正确处理该 index.ts

gulp 中对 index.ts 的 import→require 替换仅覆盖了形如 import './x' 的语句,未覆盖 import _X from './x'export ... from。请确认该文件最终是由 TS/打包器编译而非仅靠该替换逻辑生成 JS,否则可能在产物里残留 ESM 语法。

packages/sqi-web/src/tag/demos/base.tsx (1)

5-23: 示例覆盖面合理

展示了 bordered/icon/closable/自定义 close icon/阻止默认关闭等用法,便于文档演示。

packages/sqi-web/src/tag/demos/edit.tsx (1)

35-43: 确认 Input 的 onChange 形参签名

这里直接传 setInputValue,请确认组件库的 Input onChange 是否传递字符串而非原生事件,以避免 setInputValue(event)

packages/sqi-web/src/tag/type.ts (1)

2-2: 确认 ConfigSize 的导出来源

当前从 ../config-provider 导入类型,需确认该 barrel 对外 re-export 了 ConfigSize,否则应从 ../config-provider/type 引入。

packages/sqi-web/src/tag/Tag.tsx (1)

74-75: 与样式约定一致:这里使用 backgroundColor 是正确的

强调一下:color 属性作为背景色传入,与样式中 -has-color 将文本色置为对比色的设计一致。对应测试应断言 background-color 而非 color(见测试文件评论)。

Comment on lines 29 to 37
// left icon
$tag-icon-size: calc($font-size-md + 2px);
$tag-close-icon-color: $text-color-placeholder;
$tag-icon-mr: $margin-xs;

// close(right) icon
$tag-close-icon-ml: $margin-sm;
$tag-close-icon-color: $text-color-placeholder;
$tag-close-icon-hover-color: $text-color-primary;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

重复定义变量 $tag-close-icon-color,易引起混淆

该变量在“left icon”与“close(right) icon”两处均被定义,后者会覆盖前者。建议保留右侧(close 图标)处的定义,并移除前面的重复项,同时修正注释段落。

 // left icon
 $tag-icon-size: calc($font-size-md + 2px);
-$tag-close-icon-color: $text-color-placeholder;
 $tag-icon-mr: $margin-xs;
 
 // close(right) icon
 $tag-close-icon-ml: $margin-sm;
 $tag-close-icon-color: $text-color-placeholder;
 $tag-close-icon-hover-color: $text-color-primary;
📝 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
// left icon
$tag-icon-size: calc($font-size-md + 2px);
$tag-close-icon-color: $text-color-placeholder;
$tag-icon-mr: $margin-xs;
// close(right) icon
$tag-close-icon-ml: $margin-sm;
$tag-close-icon-color: $text-color-placeholder;
$tag-close-icon-hover-color: $text-color-primary;
// left icon
$tag-icon-size: calc($font-size-md + 2px);
$tag-icon-mr: $margin-xs;
// close(right) icon
$tag-close-icon-ml: $margin-sm;
$tag-close-icon-color: $text-color-placeholder;
$tag-close-icon-hover-color: $text-color-primary;
🤖 Prompt for AI Agents
In packages/sqi-web/src/tag/styles/_var.scss around lines 29 to 37, the variable
$tag-close-icon-color is defined twice (once under "left icon" and again under
"close(right) icon"), causing the first definition to be overwritten; remove the
earlier $tag-close-icon-color declaration under the "left icon" block, keep the
$tag-close-icon-color definition in the "close(right) icon" block, and update
the "left icon" comment to only include relevant variables (e.g. $tag-icon-size,
$tag-icon-mr) or rename any left-icon-specific color variable if a separate
value is required.

@@ -0,0 +1 @@
import './index.scss';
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

样式目录命名与构建脚本约定不一致,导致按需样式产物无法生成

构建脚本仅处理 src/**/style/index.ts,但本组件放在 styles/ 下,copyRenameStyleIndex 与 generateCssJs 将忽略它,导致无法生成 es/lib 下的 style/index.js 与 style/css.js,按需引入会失效。

可选修复方案(二选一,推荐 A):

  • A. 将目录由 styles 重命名为 style,并保持当前 index.ts 内容不变(推荐,和全库一致)。
  • B. 保留 styles,新增一个桥接文件 src/tag/style/index.ts 引入 ../styles/index.scss,满足脚手架扫描。

新增桥接文件示例(方案 B):

// packages/sqi-web/src/tag/style/index.ts
import '../styles/index.scss';

为避免回归,建议执行下述校验脚本确认样式产物会被生成:

我可以提交一版重命名方案(A)或桥接文件方案(B),你更偏好哪种?


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "1) 构建脚本能否捕获到 Tag 的 style/index.ts:"
fd -a 'index.ts' packages/sqi-web/src | rg -nP '/style/index\.ts$' || true

echo "2) 是否存在 styles/index.ts(当前实现):"
fd -a 'index.ts' packages/sqi-web/src | rg -nP '/styles/index\.ts$' || true

echo "3) 项目内其他组件的样式目录约定(帮助比对):"
fd -td packages/sqi-web/src | rg -nP '/style$' || true

Length of output: 1774


样式目录命名与构建脚本扫描不匹配,需修正

  • 组件目录 packages/sqi-web/src/tag/styles 与脚本仅处理 src/**/style/index.ts 不一致,导致 es/lib 下未生成对应的 style/index.jsstyle/css.js,按需引入失效
  • 修复方案(二选一,推荐 A):
    A. 将 styles 目录重命名为 style(与全库保持一致)
    B. 保留 styles,在 packages/sqi-web/src/tag/style/index.ts 新增桥接文件:
    import '../styles/index.scss';

验证示例:

fd -a 'index.ts' packages/sqi-web/src | rg -nP '/tag/style/index\.ts$'   # 确保扫描到
🤖 Prompt for AI Agents
In packages/sqi-web/src/tag/styles around line 1 the directory name "styles"
doesn't match the repository build scanner which expects "style/index.ts",
causing no style output in es/lib; fix by either (A - recommended) renaming the
folder packages/sqi-web/src/tag/styles to packages/sqi-web/src/tag/style so the
existing index.ts import is discovered by the build, or (B) keep the current
styles folder and add a bridge file at packages/sqi-web/src/tag/style/index.ts
that imports the SCSS from ../styles/index.scss so the scanner finds a
style/index.ts entry; after change, run the provided fd/rg validation to confirm
/tag/style/index.ts is detected.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/sqi-web/src/tag/demos/size.tsx (1)

7-9: 与文档语言保持一致,并标注默认尺寸(md)

当前文档章节为“尺寸”,建议 demo 文案也采用中文,并在默认项处注明“默认即 md”,便于读者理解。

-      <Tag size="sm">Small</Tag>
-      <Tag>Default</Tag>
-      <Tag size="lg">Large</Tag>
+      <Tag size="sm">小</Tag>
+      <Tag /* 默认 size="md" */>默认</Tag>
+      <Tag size="lg">大</Tag>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 52949c2 and 0b74cff.

📒 Files selected for processing (3)
  • packages/sqi-web/src/tag/demos/base.tsx (1 hunks)
  • packages/sqi-web/src/tag/demos/size.tsx (1 hunks)
  • packages/sqi-web/src/tag/index.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/sqi-web/src/tag/demos/base.tsx
  • packages/sqi-web/src/tag/index.md
🧰 Additional context used
🧬 Code graph analysis (1)
packages/sqi-web/src/tag/demos/size.tsx (3)
packages/sqi-web/src/tag/demos/base.tsx (1)
  • Demo (5-23)
packages/sqi-web/src/space/index.ts (1)
  • Space (5-5)
packages/sqi-web/src/tag/index.ts (1)
  • Tag (5-5)
🔇 Additional comments (2)
packages/sqi-web/src/tag/demos/size.tsx (2)

4-12: 演示代码清晰,API 使用正确(LGTM)

展示 sm/默认/lg 三档尺寸直观明了,与其它 demos 的 Space 用法保持一致。


1-1: 确认自动 JSX 运行时后移除 import React 并统一 demos

  • 在 node_modules 中打开 @sqi-ui/tsconfig/react-library.json(或检查 packages/sqi-web/tsconfig.build.json 的继承链),确认 compilerOptions.jsx 已设置为 "react-jsx""react-jsxdev"
  • 若已启用新运行时,可在 packages/sqi-web/src/tag/demos/*.tsx 顶部移除所有 import React from 'react';,以与其他示例保持一致。

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
packages/sqi-web/src/tag/Tag.tsx (1)

51-66: 完善 closable 渲染逻辑与可访问性(合并 className/事件,支持字符串图标,键盘可用)

  • 现在自定义图标会覆盖 className,且缺少键盘与 a11y 属性。
  • closable 类型为 boolean | ReactNode,但当传入字符串/数字时会退回默认图标,不符合类型语义。
   const renderCloseIcon = () => {
     if (!closable) return null;
 
     const closeCls = `${prefixCls}-tag-close-icon`;
 
-    if (isValidElement(closable)) {
-      return cloneElement(closable as any, {
-        className: closeCls,
-        onClick: (e: React.MouseEvent<HTMLElement>) => {
-          (closable as any).props.onClick?.(e);
-          if (!e.defaultPrevented) onInnerClose(e);
-        },
-      });
-    }
-    return <CloseIcon onClick={onInnerClose} className={closeCls} />;
+    // 1) 默认关闭图标:closable === true
+    if (closable === true) {
+      return (
+        <CloseIcon
+          onClick={onInnerClose}
+          className={closeCls}
+          role="button"
+          tabIndex={0}
+          aria-label="close"
+          onKeyDown={(e: React.KeyboardEvent<HTMLElement>) => {
+            if (e.key === 'Enter' || e.key === ' ') {
+              e.preventDefault();
+              // @ts-expect-error align event type for inner handler
+              onInnerClose(e as any);
+            }
+          }}
+        />
+      );
+    }
+
+    // 2) 自定义 React 元素
+    if (isValidElement(closable)) {
+      const props: any = (closable as any).props ?? {};
+      return cloneElement(closable as any, {
+        className: clsx(props.className, closeCls),
+        onClick: (e: React.MouseEvent<HTMLElement>) => {
+          props.onClick?.(e);
+          if (!e.defaultPrevented) onInnerClose(e);
+        },
+        role: props.role ?? 'button',
+        tabIndex: props.tabIndex ?? 0,
+        'aria-label': props['aria-label'] ?? 'close',
+        onKeyDown: (e: React.KeyboardEvent<HTMLElement>) => {
+          props.onKeyDown?.(e);
+          if ((e.key === 'Enter' || e.key === ' ') && !e.defaultPrevented) {
+            e.preventDefault();
+            // @ts-expect-error align event type
+            onInnerClose(e as any);
+          }
+        },
+      });
+    }
+
+    // 3) 原始节点(字符串/数字等)
+    return (
+      <span
+        className={closeCls}
+        role="button"
+        tabIndex={0}
+        aria-label="close"
+        onClick={onInnerClose}
+        onKeyDown={(e) => {
+          if (e.key === 'Enter' || e.key === ' ') {
+            e.preventDefault();
+            // @ts-expect-error align event type
+            onInnerClose(e as any);
+          }
+        }}
+      >
+        {closable as React.ReactNode}
+      </span>
+    );
   };
🧹 Nitpick comments (12)
packages/sqi-web/src/tag/style/_mixin.scss (1)

11-13: 避免使用 font 简写导致非预期重置

font 简写会同时重置 font-family、font-weight、line-height 等。若只需控制字号与行高,建议改为单独属性(前提:变量已定义)。

-    font: map.get($map-size, 'font');
+    // 若变量可用,建议:
+    // font-size: map.get($map-size, 'font-size');
+    // line-height: map.get($map-size, 'line-height');
packages/sqi-web/src/tag/Tag.tsx (2)

71-85: 将 {...restProps} 置于末尾,优先级交还给使用方

当前先展开 restProps,后续再次设置 title/className/style/role 等,调用方无法覆盖。建议把 restProps 放到最后。

-    <span
-      {...restProps}
-      ref={ref}
+    <span
+      ref={ref}
       title={title}
       className={classes}
       style={{ ...style, backgroundColor: color }}
-      onClick={onClick}
-      role="button"
-      tabIndex={0}
-      aria-label="close"
+      onClick={onClick}
+      role={onClick ? 'button' : undefined}
+      tabIndex={onClick ? 0 : undefined}
+      onKeyDown={
+        onClick
+          ? (e) => {
+              if (e.key === 'Enter' || e.key === ' ') {
+                e.preventDefault();
+                // @ts-expect-error align event type
+                onClick?.(e as any);
+              }
+            }
+          : undefined
+      }
+      {...restProps}
     >

9-9: defaultProps 类型声明更贴切

这里推荐用 Partial,表达“默认值的子集”,也与 useMergeProps 的签名一致。

-const defaultProps: TagProps = {};
+const defaultProps: Partial<TagProps> = {};
packages/sqi-web/src/tag/style/index.scss (6)

1-1: 移除未使用的 sass:map 引入以减小编译体积。
此文件未直接使用 map 模块能力,建议删除。

-@use 'sass:map';

11-18: 长文案的溢出处理。
white-space: nowrap; 会导致超长内容溢出;可按需增加省略或允许换行。

 .#{$module} {
   @include reset;
   display: inline-flex;
   align-items: center;
+  overflow: hidden; /* 可选 */
+  text-overflow: ellipsis; /* 可选 */
+  max-width: 100%; /* 可选:容器内使用时生效 */
   white-space: nowrap;

38-42: 避免左右图标样式相互“打架”;用选择器排除或使用 gap
当前对 .#{prefix}-icon 的通用样式会影响关闭图标,再通过更高权重回滚,建议直接排除或使用容器 gap

-  .#{$prefix}-icon {
+  .#{$prefix}-icon:not(.#{$module}-close-icon) {
     margin-right: $tag-icon-mr;
     width: $tag-icon-size;
     height: $tag-icon-size;
   }
@@
-  & &-close-icon {
+  & &-close-icon {
     margin-right: 0;
     margin-left: $tag-close-icon-ml;
-    color: $tag-close-icon-color;
-    font: $font-md;
+    color: $tag-close-icon-color;
     cursor: pointer;
     transition: color $anim-duration-base $anim-time-fn-easing;
   }

或改为在容器上使用 gap,去除针对左图标的 margin:

 .#{$module} {
   display: inline-flex;
   align-items: center;
+  gap: $tag-icon-mr;
@@
-  .#{$prefix}-icon {
-    margin-right: $tag-icon-mr;
-    width: $tag-icon-size;
-    height: $tag-icon-size;
-  }
+  .#{$prefix}-icon { width: $tag-icon-size; height: $tag-icon-size; }
+  & &-close-icon { margin-left: $tag-close-icon-ml; }

Also applies to: 45-56


49-49: 不建议在图标上使用 font 简写。
图标通常为 SVG,font 对其无效且会重置字体相关属性;若需控制大小,请依赖 width/heightfont-size

-    font: $font-md;

53-55: 补充键盘可达性样式。
为关闭图标添加 :focus-visible 提示,提升无鼠标场景可用性。

   & &-close-icon {
@@
     &:hover {
       color: $tag-close-icon-hover-color;
     }
+    &:focus-visible {
+      outline: 2px solid currentColor; /* 或使用设计令牌 */
+      outline-offset: 2px;
+    }
   }

38-42: 图标尺寸与标签尺寸联动。
当前 $tag-icon-size 为固定值,sm 下可能偏大。建议用相对单位或按尺寸映射设置。

方案 A(推荐,随字体缩放):

-  .#{$prefix}-icon { width: $tag-icon-size; height: $tag-icon-size; }
+  .#{$prefix}-icon { width: 1em; height: 1em; }

方案 B(按尺寸映射):
_var.scss 中定义 $tag-icon-size-map,并在 @include tag-size(...) 内设置对应宽高。(见下方变量文件评论的示例)

Also applies to: 59-61

packages/sqi-web/src/tag/style/_var.scss (3)

3-19: 尺寸映射可能不一致,请与设计规范核对。
md 使用 $font-sm,而 lgpadding 仍是 $padding-xs,存在“变大但留白变小/字体未增”的反直觉情况。

   md: (
-    padding: 0px $padding-sm,
-    font: $font-sm,
+    padding: 0px $padding-sm,
+    font: $font-md,
     height: $size-xs,
   ),
   lg: (
-    padding: 0px $padding-xs,
+    padding: 0px $padding-md,
     font: $font-md,
     height: $size-md,
   ),

若有 $font-lg/$size-lg,可进一步对应放大。


30-30: 用 Sass 直接计算,避免生成运行时 calc())。
$font-size-md 为绝对长度,可在编译期计算。

-$tag-icon-size: calc($font-size-md + 2px);
+$tag-icon-size: $font-size-md + 2px;

29-36: 注释与变量归类不一致。
$tag-close-icon-color 放在 “left icon” 区,易误导;建议移动到 close 区或调整注释。

-// left icon
-$tag-icon-size: $font-size-md + 2px;
-$tag-close-icon-color: $text-color-placeholder;
-$tag-icon-mr: $margin-xs;
+// left icon
+$tag-icon-size: $font-size-md + 2px;
+$tag-icon-mr: $margin-xs;
 
 // close(right) icon
+$tag-close-icon-color: $text-color-placeholder;
 $tag-close-icon-ml: $margin-sm;
 $tag-close-icon-hover-color: $text-color-primary;

此外,如采纳“尺寸联动”方案 B,可新增:

$tag-icon-size-map: (
  sm: $font-size-sm + 2px,
  md: $font-size-md + 2px,
  lg: $font-size-lg + 2px,
);

并在 @mixin tag-size($k) 内同步设置 .#{ $prefix }-icon { width/height: map.get($tag-icon-size-map, $k); }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b74cff and d6e52d2.

📒 Files selected for processing (7)
  • packages/sqi-web/src/tag/Tag.tsx (1 hunks)
  • packages/sqi-web/src/tag/__tests__/tag.test.tsx (1 hunks)
  • packages/sqi-web/src/tag/style/_mixin.scss (1 hunks)
  • packages/sqi-web/src/tag/style/_var.scss (1 hunks)
  • packages/sqi-web/src/tag/style/index.scss (1 hunks)
  • packages/sqi-web/src/tag/style/index.ts (1 hunks)
  • packages/sqi-web/src/tag/type.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/sqi-web/src/tag/tests/tag.test.tsx
  • packages/sqi-web/src/tag/type.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/sqi-web/src/tag/style/index.ts (1)
packages/sqi-web/gulpfile.js (8)
  • cssInjection (70-75)
  • copyRenameStyleIndex (81-103)
  • generateCssJs (45-60)
  • mergeCssToDist (19-27)
  • file (93-100)
  • compilerScss (8-16)
  • file (49-57)
  • compilerResetScss (30-43)
packages/sqi-web/src/tag/Tag.tsx (4)
packages/sqi-web/src/tag/type.ts (1)
  • TagProps (4-18)
packages/sqi-web/src/config-provider/context.ts (1)
  • ConfigContext (10-10)
packages/sqi-hooks/src/useMergeProps.ts (1)
  • useMergeProps (3-22)
packages/sqi-icons/src/icons/CloseIcon.tsx (1)
  • CloseIcon (4-22)
🔇 Additional comments (7)
packages/sqi-web/src/tag/style/index.ts (1)

1-1: 确认 sideEffects 已覆盖样式,无需修改

packages/sqi-web/package.json 已包含 "**/*.scss""es/**/style/*""lib/**/style/*" 等配置,可防止生产构建裁剪样式。

packages/sqi-web/src/tag/style/_mixin.scss (1)

12-12: 确认已设置 display: inline-flex,无需调整

packages/sqi-web/src/tag/Tag.tsx (2)

42-49: 关闭逻辑与 onClose 的语义处理很到位

阻止冒泡、允许 onClose 调用 preventDefault 取消关闭,再决定内部可见性,行为合理清晰。


30-38: 类名构建清晰,条件拼接合理

size/has-color/bordered 的条件类处理简洁可读。

packages/sqi-web/src/tag/style/index.scss (2)

22-33: 有色变体与关闭图标 hover 优先级处理合理。
选择器权重设计可覆盖默认关闭图标样式,符合预期。


6-6: 已验证 $prefix 定义
$prefix 已在 packages/sqi-web/src/style/_variables.scss:1 定义,无需在本文件或上游做额外声明。

packages/sqi-web/src/tag/style/_var.scss (1)

21-28: 颜色令牌选择合理。
默认与有色态令牌清晰分离,命名规范统一。

Comment on lines +7 to +15
@mixin tag-size($size) {
$map-size: map.get($tag-size, $size);

&.#{$module}-size-#{$size} {
font: map.get($map-size, 'font');
height: map.get($map-size, 'height');
padding: map.get($map-size, 'padding');
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

为未知尺寸添加显式校验,避免生成无效 CSS

当 $size 不存在于 $tag-size 时,map.get 返回 null,会输出非法属性值。建议直接在 mixin 内报错,便于及早发现问题。

 @mixin tag-size($size) {
   $map-size: map.get($tag-size, $size);
+
+  @if $map-size == null {
+    @error 'Unknown tag size: #{$size}. Available: #{map.keys($tag-size)}';
+  }
 
   &.#{$module}-size-#{$size} {
     font: map.get($map-size, 'font');
     height: map.get($map-size, 'height');
     padding: map.get($map-size, 'padding');
   }
 }
📝 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
@mixin tag-size($size) {
$map-size: map.get($tag-size, $size);
&.#{$module}-size-#{$size} {
font: map.get($map-size, 'font');
height: map.get($map-size, 'height');
padding: map.get($map-size, 'padding');
}
}
@mixin tag-size($size) {
$map-size: map.get($tag-size, $size);
@if $map-size == null {
@error 'Unknown tag size: #{$size}. Available: #{map.keys($tag-size)}';
}
&.#{$module}-size-#{$size} {
font: map.get($map-size, 'font');
height: map.get($map-size, 'height');
padding: map.get($map-size, 'padding');
}
}
🤖 Prompt for AI Agents
In packages/sqi-web/src/tag/style/_mixin.scss around lines 7 to 15, the mixin
uses map.get($tag-size, $size) without validating $size, which can return null
and produce invalid CSS; add an explicit guard at the top of the mixin that
checks map.has-key($tag-size, $size) (or if not available, check map.get !=
null) and call @error with a clear message including the invalid $size and
allowed keys when the key is missing, then proceed to read the map values only
after the validation succeeds.

Comment on lines +51 to +66
const renderCloseIcon = () => {
if (!closable) return null;

const closeCls = `${prefixCls}-tag-close-icon`;

if (isValidElement(closable)) {
return cloneElement(closable as any, {
className: closeCls,
onClick: (e: React.MouseEvent<HTMLElement>) => {
(closable as any).props.onClick?.(e);
if (!e.defaultPrevented) onInnerClose(e);
},
});
}
return <CloseIcon onClick={onInnerClose} className={closeCls} />;
};
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

❓ Verification inconclusive

补充用例覆盖键盘交互(Enter/Space)与 preventDefault

建议为默认/自定义关闭图标的键盘关闭行为与 onClose(e).preventDefault() 保留可见性的语义添加测试。

可以先用脚本查看是否已有相关用例:

如需,我可以补一段测试草案(React Testing Library + userEvent)验证键盘与 preventDefault 行为。


🏁 Script executed:

#!/bin/bash
rg -nC2 -P '(userEvent\\.keyboard|fireEvent\\.keyDown|key(Down|Up)\\()' packages/sqi-web/src/tag/__tests__/tag.test.tsx

Length of output: 194


🏁 Script executed:

#!/bin/bash
# 修复正则语法,分别匹配 userEvent.keyboard、fireEvent.keyDown、keyDown(、keyUp(
rg -nC2 \
  -e 'userEvent\.keyboard' \
  -e 'fireEvent\.keyDown' \
  -e 'keyDown\(' \
  -e 'keyUp\(' \
  packages/sqi-web/src/tag/__tests__/tag.test.tsx

Length of output: 134


补充 Tag 组件键盘可访问性测试
缺少针对 closable 模式下关闭图标的 Enter/Space 触发及 onClose(e).preventDefault() 保留可见性行为的测试(位于 packages/sqi-web/src/tag/__tests__/tag.test.tsx)。

🤖 Prompt for AI Agents
In packages/sqi-web/src/tag/Tag.tsx around lines 51-66, add missing
keyboard-accessibility tests in packages/sqi-web/src/tag/__tests__/tag.test.tsx
that verify the closable icon responds to Enter and Space and that calling
onClose(e).preventDefault() preserves visibility: write one test that renders a
closable Tag, focuses the close icon, simulates keyDown for Enter and for Space
and asserts onClose is called and the Tag is removed; write a second test where
onClose calls e.preventDefault(), simulate Enter/Space on the close icon and
assert the Tag remains visible (not removed). Ensure the tests query the close
button element (by role/label/class), fire keyboard events via userEvent or
fireEvent, and use assertions on DOM presence accordingly.

Comment on lines +78 to +81
role="button"
tabIndex={0}
aria-label="close"
>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

根元素误用可访问性属性(role/aria-label/tabIndex)导致语义错误

Tag 容器被标注为 button 且 aria-label="close",会误导读屏;同时会强行覆盖外部传入的无障碍属性。应仅在 Tag 可点击时(存在 onClick)赋予 button 语义,且不要把容器标注为“关闭”。

-      onClick={onClick}
-      role="button"
-      tabIndex={0}
-      aria-label="close"
+      onClick={onClick}
+      role={onClick ? 'button' : undefined}
+      tabIndex={onClick ? 0 : undefined}
+      onKeyDown={
+        onClick
+          ? (e) => {
+              if (e.key === 'Enter' || e.key === ' ') {
+                e.preventDefault();
+                // @ts-expect-error align event type
+                onClick?.(e as any);
+              }
+            }
+          : undefined
+      }

此外,建议将 {...restProps} 放到最后以便调用方覆盖默认属性(见下一个补丁一起调整)。

📝 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
role="button"
tabIndex={0}
aria-label="close"
>
onClick={onClick}
role={onClick ? 'button' : undefined}
tabIndex={onClick ? 0 : undefined}
onKeyDown={
onClick
? (e: React.KeyboardEvent<HTMLElement>) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
// @ts-expect-error align event type
onClick?.(e as any);
}
}
: undefined
}
>
🤖 Prompt for AI Agents
In packages/sqi-web/src/tag/Tag.tsx around lines 78-81, the Tag root incorrectly
forces accessible attributes (role="button", tabIndex={0}, aria-label="close")
which overrides caller props and misleads screen readers; change logic so these
attributes are only applied when the Tag is clickable (i.e., when onClick is
provided), remove the hard-coded aria-label="close" (let callers supply an
appropriate aria-label via props), ensure tabIndex is only set for interactive
Tags, and move {...restProps} to the end of the root element props so external
accessibility attributes can override defaults.

@json-q json-q merged commit 9092cae into main Sep 6, 2025
1 check passed
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.

1 participant