Conversation
…r DataTable The removed logic would have been useless or more harmful, because of the reasons: 1) DataTable control is designed to represent 'header' of a table, not for containing the list of DataRows. 2) Grid control is often used to lay out other controls rather than constructing a table, so there was possibilities that completely unrelated Grids would have been caught as a parent.
That case doesn't work well for the Width="Auto" column. I'd like to make DataTable control simple rather than leave the pitfall.
From the design concept, DataColumn control simply provides a space for the header content, so it should not interact with keyboard operations by default. If something requested, the DataTable user could be place any controls like a Button there.
… not present Each rows lay out their columns independent, because there's no main controller to remember the column widths.
The internal column types are increased to 5: 1. IsAbsolute: have a width that is same with DesiredWidth, or a manually resized. 2. IsAuto: have a manually resized width. 3. IsStar: have a manually resized width. 4. IsAutoFit: have a calculated width that fits to each column content of visualized row. 5. IsStarProportion: have a calculated width, from the proportion of remained spaces. DataTable assigns the width space first to 1-3, then 4-5. But, to take the actual width of 4 would need the Measure of visualized rows, so DataTable and DataRows would have mutual call of InvalidateMeasure. When the layout system is being into stable, that loop will be break.
|
@dotnet-policy-service agree |
|
Thanks @9rnsr for this PR, DataTable certainly needs a bit of love, so appreciate you diving in! There's a lot of different pieces changed here (including some formatting and variable renames) which makes it a bit tricky to review. Do you think you could do some of the following to boost this PR/process up a bit for us?
Thanks! |
|
@michael-hawker Thanks for your quick review. To make the review process more speedy, I'll split each commits into separate PR. |
|
@michael-hawker I opened independent smaller PRs #782, #783, and #784. |
|
@michael-hawker I opened two more PRs, #786 and #787. |
|
Thanks @9rnsr! I have a few other items I'm looking at currently, but the small break-outs will make it easier to start reviewing/discussing these bits independently and start merging them in over the next week! Just to confirm, we can close this PR now as it'll be superseded by the others? |
@michael-hawker No problem. |
DataColumn.IsTabStopfalse(an issue in here)DataColumn.Visibilityin the layout logic.And I removed the hybrid-case support (
DataTableHybridSample). As I commented here, it would not work onAutocolumns as expected, so we should use the pair ofDataTableandDataRowalways.