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

Border has a 1 pixel thickness even when it's thickness property is set to 0 - fix #21197

Merged
merged 7 commits into from
May 14, 2024

Conversation

kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Mar 14, 2024

Issues Fixed

Fixes #20156

Before After

@kubaflo kubaflo requested a review from a team as a code owner March 14, 2024 00:06
pjcollins
pjcollins previously approved these changes Mar 14, 2024
Copy link
Member

@pjcollins pjcollins left a comment

Choose a reason for hiding this comment

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

Fix seems reasonable to me and appears to fix the issue, though I'll defer to @jsuarezruiz for a final review.

@Eilon Eilon added the area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing label Mar 28, 2024
@rmarinho

This comment was marked as outdated.

This comment was marked as outdated.

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@dustin-wojciechowski
Copy link
Contributor

dustin-wojciechowski commented Apr 25, 2024

Tested to work on IOS, Android, and Windows:
(Windows pictured below)
image

Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, I feel this issue has been around for some time and was annoying, but now we have fixes.!

I just have a Q on the relationship between a few lines, but I am also hoping to take advantage of your PR and the nice person you are to maybe add some more tests for other permutations of the border, stroke, shape and thickness so that we don't have to write hundreds of UI tests but still protect our future selves from breaking this.

@kubaflo
Copy link
Contributor Author

kubaflo commented Apr 28, 2024

Hi, @mattleibow you're right I've added a commit with your suggested changes :)
When it comes to this part

if (StrokeShape is Shape strokeShape && StrokeThickness == 0)
{
    strokeShape.StrokeThickness = StrokeThickness;
}

I don't know why this condition is needed StrokeThickness == 0. Everything is okay without it, but I might not have tested it enough. Maybe @jsuarezruiz who introduced this line in this PR https://github.com/dotnet/maui/pull/14403/files might tell us more :)

@jsuarezruiz

This comment was marked as outdated.

This comment was marked as outdated.

@jsuarezruiz

This comment was marked as outdated.

This comment was marked as outdated.

@mattleibow

This comment was marked as outdated.

This comment was marked as outdated.

mattleibow
mattleibow previously approved these changes May 13, 2024
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

Waiting on screenshots from CI...

@mattleibow
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@mattleibow mattleibow merged commit 54c9db4 into dotnet:main May 14, 2024
49 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-border Border area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing fixed-in-8.0.60 fixed-in-9.0.0-preview.5.24307.10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Border has a 1 pixel thickness even when it's thickness property is set to 0.
8 participants