Copilot commented on code in PR #17951:
URL:
https://github.com/apache/dolphinscheduler/pull/17951#discussion_r2791056584
##########
dolphinscheduler-ui/src/views/projects/workflow/definition/index.tsx:
##########
@@ -184,16 +189,24 @@ export default defineComponent({
</Card>
<Card title={t('project.workflow.workflow_definition')}>
<NSpace vertical>
- <NDataTable
- loading={loadingRef}
- rowKey={(row) => row.code}
- columns={this.columns}
- data={this.tableData}
- striped
- v-model:checked-row-keys={this.checkedRowKeys}
- row-class-name='items'
- scrollX={this.tableWidth}
- />
+ {showSkeleton ? (
+ <NSkeleton
+ height='400px'
Review Comment:
`NSkeleton` is configured with `height='400px'` while also using
`repeat={...}`. In Naive UI, `height` applies to each repeated skeleton item,
so this will render an extremely tall placeholder (e.g., 10 * 400px). Consider
using a row-sized skeleton (or `text` skeleton) per repeat and, if you want a
fixed overall area, wrap it in a container with a fixed height instead of
setting a large per-item height.
```suggestion
```
##########
dolphinscheduler-ui/src/views/projects/workflow/definition/index.tsx:
##########
@@ -184,16 +189,24 @@ export default defineComponent({
</Card>
<Card title={t('project.workflow.workflow_definition')}>
<NSpace vertical>
- <NDataTable
- loading={loadingRef}
- rowKey={(row) => row.code}
- columns={this.columns}
- data={this.tableData}
- striped
- v-model:checked-row-keys={this.checkedRowKeys}
- row-class-name='items'
- scrollX={this.tableWidth}
- />
+ {showSkeleton ? (
+ <NSkeleton
+ height='400px'
+ repeat={this.pageSize || 10}
+ sharp={false}
+ />
+ ) : (
+ <NDataTable
+ loading={false}
+ rowKey={(row: IDefinitionData) => row.code}
+ columns={this.columns}
+ data={this.tableData}
+ striped
+ v-model:checked-row-keys={this.checkedRowKeys}
+ row-class-name='items'
+ scrollX={this.tableWidth}
+ />
Review Comment:
The table `loading` prop is now hard-coded to `false`, and the skeleton is
only shown when `tableData` is empty. When `loadingRef` is true but there is
existing data (common during search/pagination/refresh), the UI will show
neither the skeleton nor a loading indicator, which undermines the goal of
providing feedback during fetches. Consider passing `loadingRef` to
`NDataTable` when `tableData` is non-empty (or adjusting the skeleton
condition/clearing `tableData` before fetch) so users still get a loading
signal.
##########
dolphinscheduler-ui/src/views/projects/workflow/definition/index.tsx:
##########
@@ -184,16 +189,24 @@ export default defineComponent({
</Card>
<Card title={t('project.workflow.workflow_definition')}>
<NSpace vertical>
- <NDataTable
- loading={loadingRef}
- rowKey={(row) => row.code}
- columns={this.columns}
- data={this.tableData}
- striped
- v-model:checked-row-keys={this.checkedRowKeys}
- row-class-name='items'
- scrollX={this.tableWidth}
- />
+ {showSkeleton ? (
+ <NSkeleton
+ height='400px'
+ repeat={this.pageSize || 10}
Review Comment:
`repeat={this.pageSize || 10}` can become large (e.g., 30/50) based on the
pagination page size, which may render dozens of skeleton blocks and noticeably
impact performance/layout. Consider capping the repeat count to a small fixed
maximum (while still keeping the visual hint of rows).
```suggestion
repeat={Math.min(this.pageSize || 10, 10)}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
