Skip to content

feat: track updating selections with row IDs#831

Open
zamderax wants to merge 6 commits intotuist:mainfrom
zamderax:fix/updating-selectable-rowids
Open

feat: track updating selections with row IDs#831
zamderax wants to merge 6 commits intotuist:mainfrom
zamderax:fix/updating-selectable-rowids

Conversation

@zamderax
Copy link
Copy Markdown
Contributor

@zamderax zamderax commented Jan 14, 2026

  • replace rowIDs with TableRow identifiers (TableRowID) for stable selection tracking
  • add SwiftUI-style TableData(data, columns:rows:) with TerminalRow builders for Identifiable data
  • rework updating selectable table to actor-isolated state/rendering (no @unchecked Sendable)
  • update docs/examples/tests to the new API and selection behavior

@zamderax zamderax requested a review from a team as a code owner January 14, 2026 01:19
@zamderax zamderax requested review from cschmatzler and fortmarek and removed request for a team January 14, 2026 01:19
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Jan 14, 2026
@zamderax zamderax changed the title Track updating selections with row IDs feat: track updating selections with row IDs Jan 14, 2026
@zamderax
Copy link
Copy Markdown
Contributor Author

I updated the mise task headers from to to silence the deprecation warning in newer mise versions. Happy to revert if you’d rather keep the old header.

@cschmatzler cschmatzler requested a review from pepicrft January 16, 2026 16:02
Copy link
Copy Markdown
Member

@fortmarek fortmarek left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR and sorry for taking longer to review this. I wonder if we can take the opportunity to rethink the API a bit to make supporting this use case more elegant 👀

/// An interactive table that keeps updating as new data arrives.
struct UpdatingSelectableTable<Updates: AsyncSequence> where Updates.Element == TableData {
/// @unchecked Sendable: rendering and state access are serialized by internal queues.
struct UpdatingSelectableTable<Updates: AsyncSequence>: @unchecked Sendable where Updates.Element == TableData {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd love us to figure out a way not to add the @unchecked Sendable – is there a way to implement the behavior with structured concurrency constructs?

public let rows: [TableRow]

/// Optional row identifiers aligned to `rows` (used for selection tracking in updating tables)
public let rowIDs: [AnyHashable]?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

instead of a rowIDs property, I'd consider making the TableRow conform to Identifiable and basically make it more like a SwiftUI List. I'd love the API to be more like:

TableData(
  data,
  columns: columns,
  rows: [
     TerminalRow { element in
        element.title
      },
     TerminalRow { element in
        element.subtitle
      }
  ])

And if data would be Identifiable, the selection could work based on the ID (without having an explicit rowIDs property). It would require some type gymnastics to make this non-breaking, but I think it should be possible.

Wdyt? cc @pepicrft

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.

Agree. The ergonomy of the API that you suggest feels right.

@zamderax
Copy link
Copy Markdown
Contributor Author

zamderax commented Jan 23, 2026

I made some changes to actors and removed the unchecked Sendables. I also ran swiftformat and linting

@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Jan 23, 2026
@zamderax zamderax force-pushed the fix/updating-selectable-rowids branch from 72a12f6 to 7c5cf8c Compare February 1, 2026 06:00
zamderax added a commit to wendylabsinc/Noora that referenced this pull request Feb 2, 2026
@Joannis
Copy link
Copy Markdown
Contributor

Joannis commented Feb 3, 2026

@pepicrft any chance we can get a review on this? It's quite the frustrating UX right now.

@pepicrft
Copy link
Copy Markdown
Contributor

Thanks a lot for the PR @zamderax, and apologies for the delayed response @Joannis. This PR slipped through the cracks. @zamderax some comments:

  • The build and tests seem to be failing. Would you mind looking into those?
  • The placeholder rows in paginated rendering are still using [TerminalText] and not TableRow. Do you intend to update those?

updates: updates,
pageSize: 8
)
let selectedWiFiID = table.rows[selectedIndex].id.unwrap(UUID.self)
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.

Should this be?

Suggested change
let selectedWiFiID = table.rows[selectedIndex].id.unwrap(UUID.self)
let selectedWiFiID = latest.rows[selectedIndex].id.unwrap(UUID.self)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants