Maui
Maui copied to clipboard
Add DockLayout (docks views to the edges of the layout container)
Description of Change
This PR implements a DockLayout which lets you dock views to the edges (top, left, right, bottom) of the DockLayout container.
Linked Issues
[Proposal] DockLayout #107
PR Checklist
- [x] Has a linked Issue, and the Issue has been
approved(bug) orChampioned(feature/proposal) - [x] Has tests (if omitted, state reason in description)
- [x] Has samples (if omitted, state reason in description)
- [x] Rebased on top of
mainat time of PR - [x] Changes adhere to coding standard
- [ ] Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pull/XYZ (???)
Additional information
I tried to write tests, but there are several issues atm:
- Two issues/errors with the solution
Error CA1001: Type 'MaskedBehavior' owns disposable field(s) 'applyMaskSemaphoreSlim' but is not disposable
.\src\CommunityToolkit.Maui\Behaviors\MaskedBehavior.shared.cs
Error CA1001: Type 'ValidationBehavior' owns disposable field(s) 'isAttachingSemaphoreSlim' but is not disposable
.\src\CommunityToolkit.Maui\Behaviors\Validators\ValidationBehavior.shared.cs
I disabled dotnet_diagnostic.CA1001.severity = error in .editorconfig temporarily. 2. CommunityToolkit.Maui.UnitTests fails to build in VS 17.4 Preview 2 with
Error CS0016: Could not write to output file '.\src\CommunityToolkit.Maui.UnitTests\obj\GF\CommunityToolkit.Maui.SourceGenerators\CommunityToolkit.Maui.SourceGenerators.Generators.TextColorToGenerator\PublicTextStyleViewTextColorTo.g.shared.cs' -- 'Could not find a part of the path '.\src\CommunityToolkit.Maui.UnitTests\obj\GF\CommunityToolkit.Maui.SourceGenerators\CommunityToolkit.Maui.SourceGenerators.Generators.TextColorToGenerator\PublicTextStyleViewTextColorTo.g.shared.cs'.'
Unfortunately, I have no idea why this happens and what to do about it :( SourceGenerators work just fine for the 'Sample' project.
As for the issue 2, your path is too long. Max length 256 symbols. as for the issue 1, we’ll fix it
Thank you for your PR. Overall it looks good. Could you please add tests and IDockLayout interface in Core project. See example IUniformItemsLayout.
@VladislavAntonyuk: Thanks for the review (and the pointer to the MaxPath issue). I hope that I have addressed all your points.
Note: I found that UniformItemsLayout combines Layout and LayoutManager into one. Therefore, violating your own guidelines ;) From CONTRIBUTING.md:
- Layout Managers, were introduced on .NET MAUI and they live on Microsoft.Maui.Core so makes sense to have our managers on Core as well.
- Layout, will be the implementation of ours custom layouts and will use the Layout Managers on Core
Thanks @profix898! When you get a chance, could you leave a comment on the DockLayout Proposal: https://github.com/CommunityToolkit/Maui/issues/107
GitHub won't allow me to assign it to you until you've left a comment.
Thank you @profix898. I will check how it works on different platforms. If everything looks good, I will approve it.
/dotnet format
@pictos: Thanks for your feedback. I made the changes as requested/suggested.
Let me know if you have any question (:
I guess the biggest question would be the overall structure: What goes into Core? Are you happy with the interfaces? Layout vs. LayoutManager (see my note above).
Also, since updating to VS 17.4 preview 2, I have serious issues testing on Android. So I hope it still works Ok.
What goes into Core?
If a class uses the namespace Microsoft.Maui.Controls.*, it belongs in CommunityToolkit.Maui.
Otherwise, it can go in CommunityToolkit.Maui.Core
Here's more info in our Contribution Guidelines:
https://github.com/CommunityToolkit/Maui/blob/main/CONTRIBUTING.md#where-to-make-your-changes
Thanks! Should be Ok then ... Let me know if you'd like any additional changes.
I renamed DockEnum to DockPosition (and all related methods as well). Didn't use Github suggestions directly to make sure usages (incl. sample page) are updated accordingly.
Thank you @profix898, I updated the sample to show different layouts. I also moved DockPosition to the primitives folder. I also updated Spacing type from SizeF to Thickness.
Thanks @VladislavAntonyuk for your review and changes. However, I didn't use Thickness for the Spacingproperty for a reason ;) It allows you to specify separate values for Left/Top/Right/Bottom, but these values do not make any sense in the context of DockLayout IMO. We only have vertical and horizontal spacing. I think we should keep SizeF (with Width and Height) or move to two doubles (for vertical/horizontal) as suggested by @brminnick.
Maybe I incorrectly understand spacing, but what I understand it as margin of the center element. So we may have different spaces of Top,Bottom, Left,Right
You can have multiple views docked to each side (top,left,right,bottom). Currently, the spacing is applied vertically/horizontally between each of them.
For example, if you dock 2 views to the left, there is 1x horizontal spacing between the 2 docked views AND 1x horizontal spacing between the right-most docked view and the content/center view. So, spacing works as with the Grid layout, which has column/row spacing, respectively. If you want individual 'spacing' around a specific view (e.g the 'center'), you can still apply a Padding to that view.
Edit: We should have double HorizontalSpacing { get; } and double VerticalSpacing { get; } to better reflect that behavior in naming. What do you think?
Pushed a version with HorizontalSpacing/VerticalSpacing. Feel free to revert in case you disagree.
I think I addressed all your latest comments. Property is called ExpandLastChild now. Anything else?
@profix898 Thank you! I have no more comments. Could you please formatting?
Is anyone working on the documentation? Or should I give it a go?
Is anyone working on the documentation? Or should I give it a go?
Sorry I don't think it was clear in our initial comments. We usually do prefer the author of the PR to write the documentation as they have the most amount of context. That being said if you need any assistance in writing it please reach out to me and I can assist you 👍
Hey @profix898 , are you working on adding docs? Do you need any help?
Yes, I started working on documentation. Unfortunately, I'm currently quite busy with my day job. So, it might take 1-2 weeks to complete an initial PR for docs.
Added PR for docs and a new DockLayout.Add(IView, DockPosition) method to make it easier to populate the layout from C# (see docs for details).
Sidenote to MCT maintainers: It's kind of difficult to recognize a pattern of best practices from the source code. It's simply not consistent enough (yet). Two quick examples:
- UniformItemsLayout combines ILayout and ILayoutManager into a single class. From MAUI docs they should be separate. dotNET 7 MAUI even introduces a new concept (similar to Handlers) where you (as a user) can replace the LayoutManager of existing layouts (see dotnet/maui#9980), which obviously is impossible if both are combined.
- You asked me to put enums into Primitives. But for IToast the ToastDuration enum is in the same file as the interface.
- .editorconfig is still too incomplete (thus leading to inconsistent style in VS)
dotnet format(which I used to overcome problem 3) does not currently work with .NET 7 RC SDKs
@VladislavAntonyuk: You were probably 5 mins ahead of me ;) Thanks for helping out.
@profix898 thanks for your feedback, I'm working on more docs on how to contribute... Right now the best you can do is to look into our code and find some patterns. During .NET7 release we will work to make the pattern more solid
Yes thanks for the feedback. I do have an item on my list to improve the .editorconfig file so that it gives a better experience for devs, if you have any concrete examples of issues with it I would really appreciate them
@pictos: Absolutely. Keep at it. Improving consistency throughout the repo is the best (and probably only) way to help with this.
@bijington: I poked the documentation a little to better understand the issues with editorconfig. Not sure it's really correct, but my understanding is that starting with a (user-specific) default IDE configuration, the editorconfig rules are applied hierarchically to override the defaults. In that case, every options that is not specifically listed in editorconfig would use the local IDE setting instead. That would certainly, explain a lot of issues, I was seeing with this repo. For one, there are no editorconfig options for XML comments (at least I could not find any). Second, my typical code style (my defaults) appear to creep through at various places, so I suggest that there are no explicit values for these settings in your editorconfig. I'm usually using ReSharper on top (but temporarily disabled it for this), where you have very detailed control over code style (there are approx 5x more settings in R# than in editorconfig), but it additionally messes with the supposed IDE defaults. Here, I attach an editorconfig file with my IDE defaults for reference (although the export appears to be quite incomplete as well). I'm not sure which options are a problem, but maybe it helps you figuring it out!? I did find 1.5 though: csharp_space_after_cast, also blank line before returns (not sure there is an editorconfig option for that). When running dotnet format on the console, there are no IDE defaults (applies mostly for VS Code, etc. as well). For that reason, it appears that at least most styles are what you expect there. So much for my 2 cents ...
@VladislavAntonyuk As the Proposal Champion for DockLayout, I'll defer to you to merge this. If everything looks good to you, let's merge this tonight or tomorrow morning in preparation for the v1.4.0 release!
