Skip to content

refactor(ui): Update to frappe-ui latest and use its list component - #481

Merged
siduck merged 3 commits into
developfrom
ui
Oct 14, 2026
Merged

siduck merged 3 commits into
developfrom
ui

Conversation

@siduck

@siduck siduck commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator
  • Use frappe-ui's list component

@greptile-apps

greptile-apps Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[High impact] Upgrades frappe-ui and replaces table library across dashboard.

No blocking runtime defect was established; add the missing test and correct the sizing advice.

Reviews (1) · Last reviewed commit: "refactor(ui): Use frappe-ui list compone..." · Reviewed by Greptile

@previous="setPage(query.page - 1)"
@next="setPage(query.page + 1)"
@last="setPage(pageCount)"
@page-size-change="updateQuery({ pageSize: $event })"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Page-size reset lacks a test
Changing page size now resets the list to page 1, but no test covers this changed behavior. The repository requires tests for behavior changes; add a test that changes page size from a later page and checks the resulting rows.

Prompt To Fix With AI
This is a comment left during a code review.
Path: dashboard/src/components/common/list-view/ListView.vue
Line: 458

Comment:
**Page-size reset lacks a test**
Changing page size now resets the list to page 1, but no test covers this changed behavior. The repository requires tests for behavior changes; add a test that changes page size from a later page and checks the resulting rows.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

export interface ListViewColumn<TData> {
key: string
label: string
width?: string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Sizing advice names removed option
Removing size from ListViewColumn leaves the README's sizing advice wrong, so a teammate following it will use an unsupported option. Replace that advice with width and CSS grid tracks, as the repository requires existing documents to stay correct.

Prompt To Fix With AI
This is a comment left during a code review.
Path: dashboard/src/components/common/list-view/types.ts
Line: 4

Comment:
**Sizing advice names removed option**
Removing `size` from `ListViewColumn` leaves the README's sizing advice wrong, so a teammate following it will use an unsupported option. Replace that advice with `width` and CSS grid tracks, as the repository requires existing documents to stay correct.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@siduck
siduck merged commit 767ab7c into develop Oct 14, 2026
4 of 6 checks passed
@siduck
siduck deleted the ui branch October 19, 2026 07:01
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