github-starred/actual
2
0
Fork 0
mirror of https://github.com/actualbudget/actual.git synced 2026-03-22 00:13:45 -05:00
Code Issues 250 Packages Projects Releases 53 Wiki Activity

[PR #1948] [MERGED] [Maintenance] Refactor payee table to typescript #4037

New Issue
Closed
opened 2026-02-28 20:50:37 -06:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-02-28 20:50:37 -06:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/actualbudget/actual/pull/1948
Author: @kymckay
Created: 11/21/2023
Status: ✅ Merged
Merged: 11/22/2023
Merged by: @MatissJanis

Base: master ← Head: ts-payee

📝 Commits (6)

  • ccb60d8 Refactor PayeeMenu to typescript
  • e6d71ec Refactor PayeeTablRow to typescript
  • b5853d6 Harden table navigator typing
  • 54f2154 Refactor PayeeTable to typescript
  • 704a93f Add release note
  • ac8e83a Improve typing on PayeeTable forwardRef

📊 Changes

6 files changed (+113 additions, -44 deletions)

View changed files

📝 packages/desktop-client/src/components/payees/ManagePayees.js (+1 -2)
📝 packages/desktop-client/src/components/payees/PayeeMenu.tsx (+11 -1)
📝 packages/desktop-client/src/components/payees/PayeeTable.tsx (+34 -19)
📝 packages/desktop-client/src/components/payees/PayeeTableRow.tsx (+35 -6)
📝 packages/desktop-client/src/components/table.tsx (+26 -16)
➕ upcoming-release-notes/1948.md (+6 -0)

📄 Description

The diff might be hard to compare on this one as I'm factoring components out of the large index.js file (unclear why that's an index file to begin with). The main changes are:

  • Components are the same, but with typing for the props.
  • Hardening of parameters and props for the Table component and associated useTableNavigator hook. This was needed to correctly propagate types through to the table item rendering function and elsewhere that fields and IDs are exposed.

The main oddities in here:

  • The PayeeEntity type defined in loot-core's models has an optional id. That is incompatible with the TableItem type expected by the Table component. Potentially that optionality is incorrect, but rather than scour the codebase to confirm if that's the case I've just added a PayeeWithId type which makes the ID required for this component.
  • There's something incorrect about the Ref type from the Table forwarded ref. There's an existing code comment for that and I've also disabled linting where I'm pulling that type through to the ref parameter for PayeeTable too. I think the way it's typed in Table might just be wrong, but preferred to forge ahead with this incremental improvement then spend a lot of time digging into that.
  • You'll notice I'm removing the highlightedRows prop passed to PayeeTable. That's because the table was using it to fill the highlighted prop given to each table item - except that the PayeeTableRow (previously just Payee) component doesn't have or use that prop. There's some state management updating the value of that variable in the parent, but I decided to leave that in place until giving the parent a more thorough review later.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/actualbudget/actual/pull/1948 **Author:** [@kymckay](https://github.com/kymckay) **Created:** 11/21/2023 **Status:** ✅ Merged **Merged:** 11/22/2023 **Merged by:** [@MatissJanis](https://github.com/MatissJanis) **Base:** `master` ← **Head:** `ts-payee` --- ### 📝 Commits (6) - [`ccb60d8`](https://github.com/actualbudget/actual/commit/ccb60d8707399b201684e9782847642968b6142c) Refactor PayeeMenu to typescript - [`e6d71ec`](https://github.com/actualbudget/actual/commit/e6d71ec37a2216f7462df514fdc1608d3cce655c) Refactor PayeeTablRow to typescript - [`b5853d6`](https://github.com/actualbudget/actual/commit/b5853d67b0aab6524bb1871051493a7756284124) Harden table navigator typing - [`54f2154`](https://github.com/actualbudget/actual/commit/54f21547cf6817c13f241661453121ff67f645ee) Refactor PayeeTable to typescript - [`704a93f`](https://github.com/actualbudget/actual/commit/704a93f31fc31d3b5e42c25097583eb3b2f92ab3) Add release note - [`ac8e83a`](https://github.com/actualbudget/actual/commit/ac8e83ac15c468e91a84de3492a8b93a39bf550b) Improve typing on PayeeTable forwardRef ### 📊 Changes **6 files changed** (+113 additions, -44 deletions) <details> <summary>View changed files</summary> 📝 `packages/desktop-client/src/components/payees/ManagePayees.js` (+1 -2) 📝 `packages/desktop-client/src/components/payees/PayeeMenu.tsx` (+11 -1) 📝 `packages/desktop-client/src/components/payees/PayeeTable.tsx` (+34 -19) 📝 `packages/desktop-client/src/components/payees/PayeeTableRow.tsx` (+35 -6) 📝 `packages/desktop-client/src/components/table.tsx` (+26 -16) ➕ `upcoming-release-notes/1948.md` (+6 -0) </details> ### 📄 Description The diff might be hard to compare on this one as I'm factoring components out of the large `index.js` file (unclear why that's an index file to begin with). The main changes are: - Components are the same, but with typing for the props. - Hardening of parameters and props for the `Table` component and associated `useTableNavigator` hook. This was needed to correctly propagate types through to the table item rendering function and elsewhere that fields and IDs are exposed. The main oddities in here: - The `PayeeEntity` type defined in `loot-core`'s models has an optional `id`. That is incompatible with the `TableItem` type expected by the `Table` component. Potentially that optionality is incorrect, but rather than scour the codebase to confirm if that's the case I've just added a `PayeeWithId` type which makes the ID required for this component. - There's something incorrect about the `Ref` type from the `Table` forwarded ref. There's an existing code comment for that and I've also disabled linting where I'm pulling that type through to the `ref` parameter for `PayeeTable` too. I think the way it's typed in `Table` might just be wrong, but preferred to forge ahead with this incremental improvement then spend a lot of time digging into that. - You'll notice I'm removing the `highlightedRows` prop passed to `PayeeTable`. That's because the table was using it to fill the `highlighted` prop given to each table item - except that the `PayeeTableRow` (previously just `Payee`) component doesn't have or use that prop. There's some state management updating the value of that variable in the parent, but I decided to leave that in place until giving the parent a more thorough review later. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-02-28 20:50:37 -06:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
AI generated
API
bank sync
budgeting
bug
can’t replicate
dependencies
docker
documentation
electron
experimental feature
feature
feedback
goal templates
good first issue
help wanted
importers
maintenance
needs info
needs testing
needs triage
needs votes
openid
payees
pull-request
Mirrored from GitHub Pull Request
 regression
reports
responsive
rules
schedules
server
✨ merged
split transactions
tech debt
theme
transaction import
transaction reconciliation
transactions
translations
upstream
user interface
✅ approved
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/actual#4037
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?