Repository navigation
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Stringifying display-map keys breaks cached text retrieval for symbol-valued cells.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds TypeScript coverage to grid-core utilities and removes an obsolete cast.
Changes:
- Types formatting, grouping, load-panel, widget, and lookup utilities.
- Adds shared supporting interfaces.
- Simplifies
getCellText.
| File | Description |
|---|---|
grid_core/views/utils.ts |
Removes redundant string cast. |
grid_core/types.ts |
Adds utility-related types. |
grid_core/m_utils.ts |
Types and refactors grid utilities. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Alyar666
changed the title
There was a problem hiding this comment.
🟢 Approval recommended
The typing refactor preserves existing behavior and includes focused regression tests.
0 open findings
1 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
| const normalizeSortingInfo = function (sort) { | ||
| sort = sort || []; | ||
| const result = normalizeSortingInfoUtility(sort); | ||
| const normalizeSortingInfo = (sort: unknown): SortingInfo[] => { |
There was a problem hiding this comment.
We can type sort from usage, using StoreLoadOptions or its sort/group parameters types directly. Also type other utils that use same argument
| const normalizeSortingInfo = (sort: unknown): SortingInfo[] => { | |
| const normalizeSortingInfo = (sort: StoreLoadOptions['sort'] | StoreLoadOptions['group]): SortingInfo[] => { |
There was a problem hiding this comment.
There are errors with these types:
- The
isExpandedandgroupIntervalproperties are missing. equalSortParameterspasses arguments of typeunknowntonormalizeSortingInfo. The argument types will need to be properly defined throughout the call chain.
| getIndexByKey(key: RowKey, items: unknown, keyName?: string | string[] | null): number { | ||
| let index = -1; | ||
|
|
||
| if (key !== undefined && Array.isArray(items)) { |
There was a problem hiding this comment.
is there possibility items is not an array?
There was a problem hiding this comment.
It can be unknown[] | null | undefined.
b208e55 to
d9ac423
Compare
No description provided.