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

[Core] Fix Border margin issue #14402

Merged
merged 5 commits into from
Apr 20, 2023
Merged

[Core] Fix Border margin issue #14402

merged 5 commits into from
Apr 20, 2023

Conversation

jsuarezruiz
Copy link
Contributor

Description of Change

Fix Border 1px margin issue in all the platforms.

image

Applying zoom:
image

To validate the changes, launch the .NET MAUI Gallery and navigate to the Border samples. Use the included sample in this PR yo check the border margins.

Issues Fixed

Fixes #7764

@jsuarezruiz jsuarezruiz added t/bug Something isn't working area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor area-controls-border Border labels Apr 5, 2023
@jsuarezruiz jsuarezruiz mentioned this pull request Apr 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2023

Thank you for your pull request. We are auto-formatting your source code to follow our code guidelines.

@@ -312,5 +314,13 @@ void OnStrokeDashArrayChanged(object? sender, NotifyCollectionChangedEventArgs e
{
Handler?.UpdateValue(nameof(IBorderStroke.StrokeDashPattern));
}

void UpdateStrokeShape()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When using a Shape in the StrokeShape property, we didn't have the StrokeTickness mapped from the Border to the Shape.

{
if (StrokeShape is Shape strokeShape)
{
strokeShape.StrokeThickness = StrokeThickness;
Copy link
Contributor Author

@jsuarezruiz jsuarezruiz Apr 5, 2023

Choose a reason for hiding this comment

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

#3558

StrokeShape "only" needs the information to define the Shape to use for the border and to clip the content. It uses IShape so Shapes and Geometries can be used. This, apart from cause confusion, has problems:

  • Shape: In addition to specifying the geometry of the shape, it has properties to define the stroke, thickness, etc. None of these properties are used, although the thickness does impact the size of the Shape. It inherits the size of the parent so it's easy to use but have unused properties under this scenario.
  • Geometry: Designed for clipping, it only has the information to define the Path to use. It does not have properties that have no effect as in the case of Shape, but instead, you must specify the size and properties that define each type of Geometry.

To improve this situation, we should apply changes to use Geometry or, if we want to make it easier to use and avoid requiring setting sizes, etc., create a new type like IStrokeShape that defines only the Shape without having properties to manage colors, stroke, etc. In In case of choose this second option, I would also ask, does it make sense to allow using a Line for example? I think having Rectangle, RoundRectangle, Ellipse and Path covers all scenarios.

Copy link
Member

Choose a reason for hiding this comment

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

is this the only property we need to update ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, is the only one impacting in the size. The rest are not used because we don't apply colors or any other effect; just use the defined path to draw the Border.

@jsuarezruiz jsuarezruiz marked this pull request as draft April 5, 2023 09:25
@jsuarezruiz jsuarezruiz marked this pull request as ready for review April 12, 2023 12:20
@jsuarezruiz jsuarezruiz requested a review from rmarinho April 20, 2023 07:40
@rmarinho rmarinho merged commit 2b823f8 into main Apr 20, 2023
@rmarinho rmarinho deleted the fix-7764 branch April 20, 2023 16:55
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2023
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
@samhouts samhouts added the fixed-in-8.0.0-preview.4.8333 Look for this fix in 8.0.0-preview.4.8333! label Aug 2, 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.0-preview.4.8333 Look for this fix in 8.0.0-preview.4.8333! t/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Border Margin
5 participants