Skip to content

Conversation

@symbiogenesis
Copy link
Collaborator

@symbiogenesis symbiogenesis commented May 6, 2024

This would allow the BorderThickness to accurately set the real thickness.

When borders are around each cell, then the borders are effectively doubled. Setting a thickness of 1 really produces a border with a width of 2.

The performance after this change in unaffected.

@symbiogenesis symbiogenesis changed the title [Do Not Merger] Refactor to use ColumnSpacing for cell borders [Do Not Merge] Refactor to use ColumnSpacing for cell borders May 6, 2024
@symbiogenesis
Copy link
Collaborator Author

symbiogenesis commented Jul 1, 2024

@akgulebubekir What do you think about this? It is probably an improvement, at least in terms of correctness, as long as there aren't any regressions of any kind. Because setting odd-numbered thicknesses isn't possible, currently.

For performance, I haven't actually tried benchmarking the before/after with speedscope or anything.

For maintainability, this adds code and an event subscription, but the code seems slightly more readable. There might be a way to refactor it further for simplicity.

At some point if upstream MAUI performance improves, we could evaluate using a real Border control to keep things simple and maintainable. (I have been part of the effort to improve upstream MAUI performance, and progress is steady)

If you like this, let me know, and I will fix the merge conflict.

@symbiogenesis symbiogenesis reopened this Jul 1, 2024
@symbiogenesis symbiogenesis changed the title [Do Not Merge] Refactor to use ColumnSpacing for cell borders [WIP] Refactor to use ColumnSpacing for cell borders Jul 1, 2024
@symbiogenesis symbiogenesis changed the title [WIP] Refactor to use ColumnSpacing for cell borders Refactor to use ColumnSpacing for cell borders Sep 10, 2024
@symbiogenesis symbiogenesis merged commit cd3982d into akgulebubekir:main Sep 10, 2024
@symbiogenesis symbiogenesis deleted the column-spacing branch September 10, 2024 01:09
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.

1 participant