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

Include Border everywhere #650

Closed
wants to merge 43 commits into from

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Apr 5, 2021

Description of Change

Include CornerRadius and Border everywhere.

winui-customborder

Related Spec: #10

Changes:

Added to IFrameworkElement:

  • BorderShape (support also CornerRadius using RoundRectangle)
  • BorderBrush
  • BorderWidth
  • BorderDashOffset
  • BorderDashArray

Fixes:

Platforms Affected

  • Core
  • iOS
  • Android
  • Windows

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)
  • Targets a single property for a single control (or intertwined few properties)
  • Adds the property to the appropriate interface
  • Avoids any changes not essential to the handler property
  • Adds the mapping to the PropertyMapper in the handler
  • Adds the mapping method to the Android, iOS, and Standard aspects of the handler
  • Implements the actual property updates (usually in extension methods in the Platform section of Core)
  • Tags ported renderer methods with [PortHandler]
  • Adds an example of the property to the sample project (MainPage)
  • Adds the property to the stub class
  • Implements basic property tests in DeviceTests

Does this PR touch anything that might effect accessibility?

No

@jsuarezruiz
Copy link
Contributor Author

If we can create for example a Grid and apply corners, a border, shadow, etc. Does it make sense to keep the Frame in .NET MAUI or does it become a deprecated and available control, but only in the Compatibility package?

Feedback?

@AmrAlSayed0
Copy link
Contributor

Nice, I have been thinking about submitting a proposal about this for the longest time but ... I was lazy I guess 😅. Well, next step shadows (with multiple layer shadows 😮) on IFrameworkElement and I wouldn't mind multiple backgrounds with transparencies and offsets ... a true CSS parity is what I have in mind.

But ... one step at a time 🤷‍♂️.

@jsuarezruiz
Copy link
Contributor Author

@AmrAlSayed0 Well, you can take a look to the Shadows PR too #570

@ederbond
Copy link
Contributor

ederbond commented May 4, 2021

@jsuarezruiz please also implement IShadow everywhere like described here:
xamarin/Xamarin.Forms#3867
xamarin/Xamarin.Forms#1754
Maybe look into PancakeView and do the same on MAUI

@vhugogarcia
Copy link
Contributor

If we can create for example a Grid and apply corners, a border, shadow, etc. Does it make sense to keep the Frame in .NET MAUI or does it become a deprecated and available control, but only in the Compatibility package?

Feedback?

If that is the case. I personally think the frame is now deprecated.

Let's remove it.

@jsuarezruiz
Copy link
Contributor Author

Updated to support brushes:
gradient-corner-maui

@vhugogarcia
Copy link
Contributor

Awesome!! Thanks @jsuarezruiz for impplementing this. I am wondering if everywhere means also form controls such as Entry, Editors, pickers, etc.

Hopefully yes 🙏🏻

@jsuarezruiz
Copy link
Contributor Author

borders-everywhere

@Redth
Copy link
Member

Redth commented Jul 7, 2021

@jsuarezruiz was there a technical reason that made it very non-trivial to support IShape to specify the border/corners? If that's possible it would be great for customization, and we can still support ICornerRadius by applying a rounded rectangle clip shape.

@jsuarezruiz jsuarezruiz changed the title Include CornerRadius and Border everywhere Include Border everywhere Aug 25, 2021
@@ -38,7 +38,7 @@ public partial class Button : View, IFontElement, ITextElement, IBorderElement,

public static readonly BindableProperty FontAutoScalingEnabledProperty = FontElement.FontAutoScalingEnabledProperty;

public static readonly BindableProperty BorderWidthProperty = BindableProperty.Create("BorderWidth", typeof(double), typeof(Button), -1d);
public static readonly new BindableProperty BorderWidthProperty = BindableProperty.Create("BorderWidth", typeof(double), typeof(Button), -1d);
Copy link
Member

Choose a reason for hiding this comment

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

Can we just delete all of these? What's the reason to keep any of these around now that the base class has this same property with the same type

Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

Setting BorderWidth to zero makes the view disappear

image

@PureWeen
Copy link
Member

#2253

@jsuarezruiz
Copy link
Contributor Author

Setting BorderWidth to zero makes the view disappear

image

Will review this case in all the platforms. In theory, if you set the BorderWidth to 0, are mostly removing the custom border and drawing again the native control specific border. Take a look to the gif included in the first message.

Seems to be Android?. Anyway, will review in all the platforms this case.

@PureWeen
Copy link
Member

PureWeen commented Aug 25, 2021

#2261
#2262

# Conflicts:
#	src/Core/src/Handlers/View/ViewHandler.cs
#	src/Core/src/Platform/Android/ViewExtensions.cs
/// Gets a collection of Double values that indicate the pattern of dashes and gaps
/// that is used to outline shapes.
/// </summary>
double[] BorderDashArray { get; }
Copy link
Member

Choose a reason for hiding this comment

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

It seems like this needs to be reconciled with IShapeView because IShapeView also has APIs that seem to match these ones. This causes ShapeView to have

       public float[]? StrokeDashPattern => throw new System.NotImplementedException();
        public double[] BorderDashArray => throw new System.NotImplementedException();

@@ -21,4 +20,19 @@ public DoubleCollection(double[] values)
public static implicit operator DoubleCollection(float[] f)
=> f == null ? new() : new(Array.ConvertAll(f, x => (double)x));
}

public static class DoubleCollectionExtensions
Copy link
Member

Choose a reason for hiding this comment

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

Is ToArray() not working here?

@@ -14,7 +15,8 @@ sealed class Style
}

public IDictionary<string, string> Declarations { get; set; } = new Dictionary<string, string>();
Dictionary<KeyValuePair<string, string>, object> convertedValues = new Dictionary<KeyValuePair<string, string>, object>();
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should be a tuple (x, y) so they can be named and also show what this is?

# Conflicts:
#	src/Controls/samples/Controls.Sample/Pages/Controls/ButtonPage.xaml
@mattleibow
Copy link
Member

The one downside of wrapping or clipping for a border is that the controls break if they have their own system.

This is fine for things like layouts, labels or any other elements that do not natively have "chrome". However, some controls already have this and might need customization for each, or maybe should use the native features. This will also be noticeable on windows 11 when we get the rounded corners. If I set the corner radius to less than the default, then you will see it through/behind the overlay border.

Things like text boxes are also a bit different. In most cases, you can remove the borders and things. but this is not the same as just adding a border. The underlying control needs adjustments.

border

@jsuarezruiz
Copy link
Contributor Author

Have sense to allow apply a custom border in any View?. For example, can create a custom border setting the Stroke properties in a Shape.

@vhugogarcia
Copy link
Contributor

Have sense to allow apply a custom border in any View?. For example, can create a custom border setting the Stroke properties in a Shape.

I think it makes sense. Because that will bring the ability to design UIs that could render a view kids friendly. Just thinking on that at first glance of course.

@mattleibow mattleibow changed the base branch from main to release/6.0.1xx-rc1 August 27, 2021 02:39
@vhugogarcia
Copy link
Contributor

Hello @jsuarezruiz, I hope you are good! 😀. Excuse me, I am wondering if on .NET MAUI the form controls such as entries, editors, pickers, etc will feature padding property. Due to when we change the border corner for example on Entries or editors the inner text and placeholder cannot be set a padding to accomodate the text inside the form control.

Just curiosity.

Thanks in advance for the amazing team work!!!

@AmrAlSayed0
Copy link
Contributor

@rmarinho What happened?

@rmarinho
Copy link
Member

@AmrAlSayed0 we decided to go with #2445 , so we will have a Border control instead of Border in any control.

@mattleibow mattleibow deleted the cornerradius-everywhere branch April 4, 2022 16:50
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.