[Feature-17948][ui] Add loading skeleton/placeholder for workflow definition list during data fetch #17951
Conversation
…inition list during data fetch
|
Thanks for opening this pull request! Please check out our contributing guidelines. (https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md) |
…nscheduler into Feature-17948
|
SbloodyS
left a comment
SbloodyS
left a comment
There was a problem hiding this comment.
Please don't directly use AI generated code.
… during data fetch
…nscheduler into Feature-17948
No i am not directly using AI Generated Code , but i am taking help from it. |
There was a problem hiding this comment.
Pull request overview
Adds a loading skeleton to the workflow definition list page so users get immediate visual feedback while the table data is being fetched (initial load and other cases where the table is currently empty), addressing #17948.
Changes:
- Import and render
NSkeletonas a placeholder whenloadingRefis true and the table has no rows. - Disable
NDataTable’s built-inloadingspinner by hard-codingloading={false}. - Add a stronger type annotation to the table
rowKeyfunction viaIDefinitionData.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {showSkeleton ? ( | ||
| <NSkeleton | ||
| height='400px' | ||
| repeat={this.pageSize || 10} |
There was a problem hiding this 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).
| repeat={this.pageSize || 10} | |
| repeat={Math.min(this.pageSize || 10, 10)} |
| <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} | ||
| /> |
There was a problem hiding this 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.
| /> | ||
| {showSkeleton ? ( | ||
| <NSkeleton | ||
| height='400px' |
There was a problem hiding this 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.
| height='400px' |
|
@SbloodyS i have fix all the copilot issues, please review it now |
2 participants
What changes were proposed in this pull request?
This PR adds a loading skeleton/placeholder for the workflow definition list during data fetch operations, addressing issue #17948.
Changes:
NSkeletoncomponent from Naive UI to display loading placeholderloadingprop tofalseto prevent triggering global loading indicatorsImplementation Details:
loadingRef === trueANDtableDatais empty/nullBenefits:
Related issue(s)
Closes #17948
Type of change
Testing
Checklist: