Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TableView - adds last column dividing line #1289

Merged
merged 7 commits into from
May 11, 2021
Merged

Conversation

tznind
Copy link
Collaborator

@tznind tznind commented May 7, 2021

There are a couple of issues I need to iron out (see demo) and a unit test to add but is this a suitable fix for #1286 ?

table-last-column

@BDisp
Copy link
Collaborator

BDisp commented May 7, 2021

But with an option if the last column width is not set, then it fill all the available width.
For example the DataGridColumn if width is * it fill with auto size.

@tig
Copy link
Collaborator

tig commented May 7, 2021

This meets my needs.

@tznind
Copy link
Collaborator Author

tznind commented May 7, 2021

Ok cool I will try to work out the issues and make it only behave this way when there is a MaxLength set but it might take me a few days to get to this.

@tznind
Copy link
Collaborator Author

tznind commented May 10, 2021

So MaxWidth is an int so always has a value (it also has a configurable default). Rather than change this or impose some arbitrary logic I have just added a style setting EnforceMaxWidthOnLastColumn:

I have also added tests. In the interests of 'no surprises' backwards compatibility I have left it as default false (do not have the last column). If you think this should instead default to true let me know and I can change that.

enforceMaxWidth
Toggling EnforceMaxWidthOnLastColumn in the TableEditor scenario in UICatalog

@tznind tznind marked this pull request as ready for review May 10, 2021 11:11
@BDisp
Copy link
Collaborator

BDisp commented May 10, 2021

How about if MaxWidth is equal to -1 then the column is hidden, if equal to 0 (zero), automatically sets the EnforceMaxWidthOnLastColumn to true.

@tznind
Copy link
Collaborator Author

tznind commented May 10, 2021

So the way the table view works is:

  • Take y rows of data (scroll offset + height - space taken by headers)
  • Calculate width of first column (maximum width of the content of those rows)
    • Enforce MinWidth
    • Enforce MaxWidth
  • If there is space for the column add it to the list of columns that will be rendered

This has several advantages

  • As you scroll down a large table the columns auto resize to allow the content to fit
  • We don't have to resolve the entire table into text in order to render it
  • You never see half a column being rendered

So I'm not sure MaxWidth is exactly related to this last column feature. Given that with small content the last column is going to be less width than MaxWidth anyway.

Maybe It would be better to rename this Style setting TableStyle.ExpandLastColumn*? (certainly that is a better name than EnforceMaxWidthOnLastColumn now I think about it).

*edit: Clarified that it is a TableStyle setting not anything more general

@BDisp
Copy link
Collaborator

BDisp commented May 10, 2021

Maybe It would be better to rename this Style setting Style.ExpandLastColumn? (certainly that is a better name than EnforceMaxWidthOnLastColumn now I think about it).

Well, that depends. If EnforceMaxWidthOnLastColumn can be used by any Style, it can also cover other styles.

Just a question, can it have a hidden column?

Thanks for your efforts.

@tznind
Copy link
Collaborator Author

tznind commented May 10, 2021

Just a question, can it have a hidden column?

Thanks for your efforts.

Not in the TableView API. But you can use System.DataTable.RemoveColumn(DataColumn col) to take a column out and you can add it back in again with System.DataTable.AddColumn(DataColumn col). So it would be very easy for the API user to keep a list of removed ones and simulate hiding/showing columns.

@BDisp
Copy link
Collaborator

BDisp commented May 10, 2021

Understood. Many thanks @tznind :-)

@BDisp
Copy link
Collaborator

BDisp commented May 10, 2021

Well, that depends. If EnforceMaxWidthOnLastColumn can be used by any Style, it can also cover other styles.

Sorry @tznind, my bad. EnforceMaxWidthOnLastColumn already belong to Style. I failed that.

Copy link
Collaborator

@tig tig left a comment

Choose a reason for hiding this comment

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

Love.

@tig tig merged commit 0dd0f2e into gui-cs:main May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants