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

Avoid negative MaxWidth values #28

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

XamlBrewer
Copy link
Contributor

TableViewCell MeasureOverride can be called by the platform before the control is rendered. This happens for example when the ItemsSource is defined declaratively and column calculations are started before the page is loaded. Your sample app doesn't experience this, since it waits for a Loaded event from its hosting grid.

When MeasureOverride is called on a tableview that is not yet rendered, then Column.ActualWidth is still zero, and the element is given a negative MaxWidth value. This causes the app to crash.

NegativeMaxWidth

MeasureOverride can be called during rendering, even before the control is rendered. This happens for example when the ItemsSource is defined declaratively. At that time Column.ActualWidth is still zero, and element is given a negative MaxWidth value. This causes the app to crash.
Copy link
Owner

@w-ahmad w-ahmad left a comment

Choose a reason for hiding this comment

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

Good catch, @XamlBrewer! The if condition you added ensures that element.MaxWidth is only set to a positive value, which is great. However, I believe we can simplify this logic further by using the Math.Max function. This approach ensures element.MaxWidth is never negative and will be zero if the calculated width is negative, all in a more concise manner.

Instead of:

if (contentWidth > 0)
{
    element.MaxWidth = contentWidth;
}

We can use:

element.MaxWidth = Math.Max(contentWidth, 0);

Using Math.Max eliminates the need for an additional else block, thereby reducing the lines of code and maintaining clarity.

What do you think?

@XamlBrewer
Copy link
Contributor Author

Actually I prefer my proposed implementation: only assign MaxWidth when it's relevant, otherwise don't touch it and let the basic layout routine do its work. Programmatically setting MaxWidth to zero basically makes the column or the cell invisible, and that might not be what you want. At the end of the day, users probably prefer suboptimal column auto-widths over disappearing colums (with MaxWidth zero).

Alternatively, you can indeed use Math.Max but then I would suggest to use another threshold value (something greater than zero) as a kind of 'Minimum MaxWidth' that would keep the column at least visible and selectable.

@w-ahmad
Copy link
Owner

w-ahmad commented Jul 17, 2024

When the calculated width is negative, the original problem reoccurs. To replicate this issue, set both MinWidth and MaxWidth of any column to 20 or less in the sample app. Here is the result...
image
The cell content should be entirely hidden since its left and right margins are set to 12, requiring more than 24 pixels to be visible. Thus, there isn't enough space for the text to display like the Column Header. Setting MaxWidth to 0 doesn't solve the problem. An alternative solution might be adding an else block with element.Visibility = Visibility.Collapsed.
Although it's not an ideal fix, but a "Hack". If there's another solution to this issue, I welcome its implementation.

@w-ahmad
Copy link
Owner

w-ahmad commented Jul 17, 2024

Approved the PR because it resolves the negative width assignment exception.

@XamlBrewer
Copy link
Contributor Author

I agree that the fix doesn't solve UI issues, it only prevents a crash.

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.

2 participants