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

feat: Table组件支持配置多个fixed #333

Merged
merged 2 commits into from
Jun 19, 2023
Merged

feat: Table组件支持配置多个fixed #333

merged 2 commits into from
Jun 19, 2023

Conversation

wanchun
Copy link
Collaborator

@wanchun wanchun commented Jun 19, 2023

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

Related issue (if exists):

Other information:

fixLeft?: boolean;
fixRight?: boolean;
fixedLeft?: boolean;
fixedRight?: boolean;
colSpan?: number;
rowSpan?: number;
children?: ColumnInst[];

Choose a reason for hiding this comment

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

这段代码看起来只是一个重命名变量的更改,并没有改变属性的实际用途或修改它们的类型。原变量名分别为 "fixLeft" 和 "fixRight",现在被更名为 "fixedLeft" 和 "fixedRight"。

建议将其他文件中此接口的所有实例都更新一遍,以确保代码库内部一致性。

若有其他文件在使用这些属性,则不得不更新或者创建一个别名,以便兼容旧代码。

{!props.composed && rootProps.showHeader && <Header />}
{!props.composed && rootProps.showHeader && (
<Header columns={props.columns} />
)}
{renderBody()}
</table>
);

Choose a reason for hiding this comment

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

这段代码是一个 Vue.js 组件的渲染函数,用于渲染表格组件。代码本身没有明显的错误或风险,只是进行了一个小修改,将 <Header /> 组件的属性 columns 传递给它,以便在需要时使用。此外,composed 属性决定是否渲染表头,如果为 true 将不会显示表头。

对于代码的改进建议,我建议增加对每个属性和方法的注释说明,以便维护者更快速地理解代码的作用和实现方式。此外,如果可能的话,可以对代码进行进一步的重构,以提高可读性和可维护性。

]}
style={getCustomCellStyle({ column })}
onClick={($event) => {
handleHeaderClick({ column }, $event);
handleClickSortHeader({ column });

Choose a reason for hiding this comment

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

这个代码补丁主要是在一个 Vue.js 组件中增加了一个名为 columns 的 Props 属性,并更新了其中的一些方法,使它们能够使用新的 Props 中的值。

在代码的改进方面,建议:

  • 需要添加注释来描述组件Props和方法的作用与参数。
  • 注意代码的缩进风格应该保持一致,可使用一种常见的规范化工具格式化代码,从而使其更易读并避免错误出现。
  • 可以将 getCellStyle()getCustomCellStyle() 方法合并成一个方法,以减少冗余代码。

@@ -41,7 +41,7 @@ export default defineComponent({
cellpadding="0"
>
<Colgroup columns={props.columns} />
<Header />
<Header columns={props.columns} />
</table>
</div>
);

Choose a reason for hiding this comment

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

这段代码是使用React定义了一个名为"Table"的组件,且该组件内部包含一个以props.columns作为列数的表头和数据显示区域。

在代码中的修改建议对Header组件进行了调整, 将其原本无参数传入改为传入props.columns,进一步增加了组件的可复用性。

具体来说,存在以下潜在问题:

  • 表格的单元格(td或th)是否正确设置了应有的属性,如rowspan/colspan、其它aria/accessibility属性等?
  • 表格结构上是否按照最佳实践来进行设计,如是否使用了语义化标签、表格头是否通过scope、id、headers等方式与其它单元格进行关联、表格是否支持响应式布局等?
  • 组件是否需要对来自父组件的prop值做一些类型检查或安全性检查?

我们建议在进行更多测试和使用前,开发人员还应该完善文档并尽可能地考虑使用CSS样式来控制表格的呈现,以及在必要情况下将thead/tfoot/tbody分离出来。

column,
columnIndex,
columns: props.columns,
}),
...getCustomCellClass({
row,
column,

Choose a reason for hiding this comment

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

该代码补丁添加了一个处理表格单元格的列样式和值的函数。这些函数似乎都依赖于props.columns数组使用,因此将其添加为一个prop类传递。

除此之外没有显而易见的bug风险。一些改进建议:

  1. 对于getCellStyle() 和 getCustomCellStyle() 来说,可以考虑将css样式从对象中提取出来并在css class中定义,以获得更好的组件风格和性能。
  2. 尝试给code块增加注释以提高可读性和可维护性。
  3. 有必要评估是否需要为嵌套层级过多的组件重构引入另外的设计模式,以避免复杂性或过度耦合的问题。

@@ -45,6 +45,7 @@ export default defineComponent({
rowIndex={rowIndex}
column={column}
columnIndex={columnIndex}
columns={props.columns}
onClick={($event: Event) => {
handleCellClick(
{

Choose a reason for hiding this comment

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

这段代码主要的变动是添加了一个新的属性 columnsTableBodyCell 组件,以及在组件中使用该属性。根据提交的 diff 来看,这个变动似乎没有引入新的问题或者风险。但是由于我们只能看到部分的代码,可能需要对其他的地方做更多的考察才能确定这个改动的安全性。

另外,从代码来看,TableBodyCell 组件似乎缺少必要的类型定义文件或者类型声明,可以尝试补充这方面的内容。

@wanchun wanchun merged commit c24ed9c into main Jun 19, 2023
1 check passed
@wanchun wanchun deleted the feat-table branch June 19, 2023 12:24
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