Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A remaining AOT-incompatible row-template binding and duplicate-column header regression must be addressed.
Review effort: Balanced
Findings: 1
Open (4)
What changed in this PR
Removes generated bindable-property metadata from library controls and replaces affected bindings for Native AOT compatibility.
Changes:
- Replaces internal reflective bindings with direct assignments or dependency-property bindings.
- Adds refresh callbacks for date/time formatting and header changes.
- Converts group key/count values to dependency properties and updates tests.
| File | Description |
|---|---|
tests/TableViewTimeColumnTests.cs |
Tests time formatting and refresh behavior. |
tests/TableViewDateColumnTests.cs |
Tests date formatting and refresh behavior. |
src/TableViewRowPresenter.cs |
Binds row-header dimensions directly to TableView. |
src/TableViewRowHeader.cs |
Removes generated bindable metadata. |
src/TableViewRow.cs |
Refreshes formatted cells and replaces cell dimension bindings. |
src/TableViewHeaderRow.cs |
Assigns column-header content directly. |
src/TableViewGroupHeaderRow.cs |
Exposes group information for XAML metadata. |
src/TableViewCell.cs |
Removes generated bindable metadata. |
src/TableView.cs |
Binds row fonts directly to the owning table. |
src/ItemsSource/TableViewGroupInfo.cs |
Converts key and count to dependency properties. |
src/Columns/TableViewToggleSwitchColumn.cs |
Removes generated bindable metadata. |
src/Columns/TableViewTimeColumn.cs |
Directly applies and refreshes clock formatting. |
src/Columns/TableViewTextColumn.cs |
Removes generated bindable metadata. |
src/Columns/TableViewTemplateColumn.cs |
Removes generated bindable metadata. |
src/Columns/TableViewNumberColumn.cs |
Removes generated bindable metadata. |
src/Columns/TableViewHyperlinkColumn.cs |
Removes generated bindable metadata. |
src/Columns/TableViewDateColumn.cs |
Directly applies and refreshes date formatting. |
src/Columns/TableViewComboBoxColumn.cs |
Directly configures generated ComboBox editors. |
src/Columns/TableViewColumn.cs |
Synchronizes header content through a callback. |
src/Columns/TableViewCheckBoxColumn.cs |
Removes generated bindable metadata. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+393
to
+394
| Path = new PropertyPath(nameof(TableView.RowHeight)), | ||
| Source = TableView |
Comment on lines
+570
to
+572
| if (d is TableViewColumn column && column.HeaderControl is not null) | ||
| { | ||
| column.HeaderControl.Content = e.NewValue; |
Comment on lines
+77
to
+78
| column.DateFormat = "longdate"; | ||
| column.RefreshElement(cell, new ColumnTestItem()); |
Comment on lines
+68
to
+69
| column.ClockIdentifier = "24HourClock"; | ||
| column.RefreshElement(cell, new ColumnTestItem()); |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Removes
[WinRT.GeneratedBindableCustomProperty]from every class in the TableView library (13 types) and replaces the internal bindings that depended on it. The library now works under Native AOT and JIT without the attribute. Sample and test app models are unchanged.Why the attribute was needed: under Native AOT, WinUI
{Binding}resolves a property by name only if it is a dependency property on a type in the XAML type metadata, or if the type has the attribute. Anything else falls back toICustomPropertyProvider, which throwsNotSupportedExceptionand crashes the app. The library relied on this in three places:TableViewRow,TableViewCellandTableViewRowHeaderbound through their plainTableViewproperty ("TableView.RowHeight","TableView.FontSize", ...).{Binding Key}/{Binding Count}to plain properties onTableViewGroupInfo.Changes:
Source = TableViewtoTableViewDPs.Contentis set directly, and a newHeaderchanged callback keeps it in sync.DateFormat/ClockIdentifierchanges reach realized cells viaHandleColumnPropertyChanged→ row →RefreshElement(new overrides).ItemsSource,SelectedValuePath,DisplayMemberPathandIsEditableassigned directly, matching the date/time picker editors.TableViewGroupInfo.Key/Countare now DPs (addsKeyProperty/CountProperty). A new publicTableViewGroupHeaderRow.GroupInfoputsTableViewGroupInfointo the library's XAML metadata, so{Binding Key}/{Binding Count}keep working in default and custom group header templates under AOT.DateTimeFormatHelper.Formatnow assert the format value, and new tests cover refreshing it.Related Issue
Closes #
Type of Change
Checklist
mainbranchScreenshots / Recordings
N/A: no visual changes.
Additional Notes
Testing
Build: the library builds on all six targets (net8/9/10, Windows and Uno) with no new warnings. The sample app and
AotTestAppbuild.Unit tests: 366/366 pass.
Runtime: a probe app, published with Native AOT and also run with JIT, each with columns declared in XAML and created only in code, checked:
{Binding}and{x:Bind}) group header templatesAll checks pass with no binding errors, except the pre-existing ComboBox issue listed below.
Uno: built only, not run. The attributes were Windows-only, and the Uno-side changes are limited to
Source = TableViewbindings and direct assignments.Behaviour changes
Native AOT apps: app XAML that uses
{Binding}to reach plain properties of these library types now crashes. Examples:{Binding TableView.RowHeight, RelativeSource={RelativeSource TemplatedParent}}in a custom cell/row styleColumn.*,IsCurrent,CellsDataContextwhen the column is created in codeUse
x:Bind,TemplateBinding, or bind toTableViewDPs instead. Worth a release-notes entry.Interface removed: the 13 types no longer implement the CsWinRT-generated
IBindableCustomPropertyImplementation.ComboBox editor: no longer updates live if the column's properties change mid-edit.
Non-AOT apps: no change expected.
Docs: unchanged. The guidance that user data models need the attribute still holds.
Known pre-existing AOT issues (unchanged here, tracked separately)
List<T>/ObservableCollection<T>to the ComboBox editor'sItemsSourcethrows under AOT;v1.5.0fails the same way.AutoGenerateColumnsproduces no columns, and grouping by property name puts every item under a null key under AOT, even with an attributed model.