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 control #2445

Merged
merged 7 commits into from
Sep 24, 2021
Merged

Border control #2445

merged 7 commits into from
Sep 24, 2021

Conversation

hartez
Copy link
Contributor

@hartez hartez commented Sep 10, 2021

Proposing this as an alternative version of #2347

This uses 95% of the same code, it just moves it into a custom Border control rather than having it on Layout.

Rationale

I'm presenting this as an alternative to adding border-ish properties to the existing Layout controls. The idea is to have a control which does exactly one thing: put a border around "content". The content can be any other control - a simple Label or Button, a Layout, a ContentView, or even another Border. It does one thing and it does that one thing well. It prevents bloat on Layout and gives us something that's more flexible and easier to maintain.

Is there any precedent for this?

Yes - see WPF/UWP/Silverlight/WASDK, which all provide a Border control. See also the Xamarin.Forms Frame control. In fact, we could use this to implement the Frame control for compatibility. So this idea is one that users of all those SDKs will be familiar with.

Doesn't this add more controls to the hierarchy?

When compared to the Layout solution, it depends. If the goal is to add a border to a single control, like a Label - it's a wash. You're either adding a Border or you're adding a Layout.

If the goal is to add a border around a group of controls, then yes - you're adding another item to the hierarchy (Border -> Layout -> [controls in layout]).

Wait, isn't that bad?

Meh. Again, it's a wash if you're talking about putting a border on a single control or a ContentView. When putting a border on a Layout, there's some impact.

In Forms, every extra level of the hierarchy was bad because it really meant 2 levels - one for the renderer, and one for the actual native control (leaving aside Android fast renderers, which only added a single layer). MAUI doesn't have this issue - Handlers don't exist in the native control hierarchy. Additionally, MAUI does fewer measurement/layout passes than Forms did. The impact of adding another control (which only draws a border) is much less.

Also, there's an opportunity here for future optimization. On Windows, we may be able to defer all of the work to the native (and highly optimized) Border control. On Android and iOS, we may be able to pull the extra control out of the hierarchy entirely and simply push the background layers down to the content.

Are there any downsides to the Layout approach?

It bloats the Layout type with a ton of properties that are only useful if you want a border. And most of the time with Layouts, you don't. Moving those properties to a Border control keeps everything cleaner and simpler to maintain.

And let's say you want a border around a single control, like a Button. If only Layouts have border-ish properties, then you'll have to wrap that Button in a Layout. Which Layout will you choose? Will your XAML include StackLayouts or Grids with single controls in them just to support borders? That's semantically awkward. Or will you create your own custom Layout which only lays out a single child? Congrats; you've just duplicated the Border control!

/// <summary>
/// Define how the Shape outline is painted on Layouts.
/// </summary>
public interface IBorderStroke : IStroke
Copy link
Member

Choose a reason for hiding this comment

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

Should this maybe be IStrokeShape : IShape, IStroke?

Then ShapeDrawable can take IShape and then check for shape is IStroke? Then optionally add strokes.

Copy link
Member

Choose a reason for hiding this comment

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

@rmarinho this will help with the indicator view as well. You should be able to pass the shape in via the property nad it just magically works.

@rmarinho rmarinho mentioned this pull request Sep 22, 2021
12 tasks
@Redth Redth merged commit cdf4d4e into main Sep 24, 2021
@Redth Redth deleted the border-control branch September 24, 2021 00:39
@Redth Redth mentioned this pull request Sep 24, 2021
2 tasks
@Redth Redth added this to the 6.0.101-preview.9 milestone Sep 27, 2021
@ghost ghost added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Jul 20, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 24, 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-6.0.101-preview.10 Look for this fix in 6.0.101-preview.10! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants