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

[Proposal] Project Reorganization #213

Closed
9 tasks done
pictos opened this issue Dec 8, 2021 · 25 comments · Fixed by #214
Closed
9 tasks done

[Proposal] Project Reorganization #213

pictos opened this issue Dec 8, 2021 · 25 comments · Fixed by #214
Assignees
Labels
approved This Proposal has been approved and is ready to be added to the Toolkit champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature documentation approved ♻ housekeeping This issue/PR is related to internal stuff.
Milestone

Comments

@pictos
Copy link
Member

pictos commented Dec 8, 2021

Project Reorganization

  • Proposed
  • Prototype: Not Started
  • Implementation
    • iOS Support
    • Android Support
    • macOS Support
    • Windows Support
  • Unit Tests
  • Sample

Link to Discussion

Summary

We will change the project structure to mimic the .NET MAUI structure, placing all code that doesn't use Microsoft.Maui.Controls into a new project, CommunityToolkit.Maui.Core.

This creates a new NuGet Package, CommunityToolkit.Maui.Core that new Toolkits like CommunityToolkit.Comet can leverage.

Motivation

Make our implementation as close as possible from .NET MAUI implementation, which will make it possible to easily support frameworks that are built on top of .NET MAUI and promote our features to the .NET MAUI.

Detailed Design

image

CommunityToolkit.Maui.Core

In general, this project will have all the basement to develop our Toolkit, including some primitive types, interfaces and base classes, base views, and common code. This will be referenced by other Frameworks/Toolkit based on .NET MAUI that wants to have the same features that us.

Here we will have some:

  • BaseViews, could be Views that will be used by other Views, like PaddingButton (that's used by Snackbar) or the MCTPopup that will be a native control implemented in a way that can work with our handler. This same approach is used here

  • Primitives, which will be base types that can be used by everyone, like our MathOperator. So other frameworks may not have the concept of Behavior or Converter but they can mimic them as helper classes/methods and use our primitives.

  • Common Code, this will be all generic code (platform-specific or not) that can be used by other Frameworks/Toolkits

  • 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.

  • Handlers, on Core will be the most general Handler with the majority of features.

CommunityToolkit.Maui:

This project has a reference to the Core project. Here will live the implementation of our Controls, Views, Behaviors, Animations, etc. In other words, this project will work with the .NET MAUI and will be MVVM friendly. Also, other Toolkits/Frameworks can reference this package if needed.

Here we will have some:

  • View Implementation, with BindableProperties, support to attach effects, behaviors, triggers, and all that jazz.

  • Platform Configuration, that is Platform-specific features, that can relate to some control - like the ArrowDirection that is part of Popup and works just on iOS - or the application itself - like the StatusBarColorEffect from XCT.

  • Handlers Implementation, We will add to our PropertyMapper and/or CommandMapper any Platform Configuration that some Handler/View may have. We also can implement here some features that we think will not be great to have on Core. Here is a reference for this

  • Layout, will be the implementation of ours custom layouts and will use the Layout Managers on Core

Drawbacks

  • Adds an additional NuGet packages
    • Requires adjustments to the CI Pipeline to publish two Nuget Packages
    • Additional dependencies increase compilation
  • Reorganizing the entire project may lead to unintentional breaking changes
    • This work requires touching every file; some files may be forgotten, some files may be miscopied
@pictos pictos added new proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail labels Dec 8, 2021
@pictos pictos self-assigned this Dec 8, 2021
@pictos pictos added approved This Proposal has been approved and is ready to be added to the Toolkit ♻ housekeeping This issue/PR is related to internal stuff. and removed proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail labels Dec 8, 2021
@TheCodeTraveler
Copy link
Collaborator

TheCodeTraveler commented Dec 9, 2021

I see two benefits to rearchitecting the repo:

  • More-easily promote our code into the .NET MAUI repo if the namespaces are aligned
  • Share code with future Toolkits, like CommunityToolkit.Comet and CommunityToolkit.Blazor

I prefer to create a new repository for future frameworks such as CommunityToolkit.Maui.Comet and CommunityToolkit.Maui.Blazor so that we can keep their DevOps pipelines and release schedules separate from CommunityToolkit.Maui, similar to how we are handling CommunityToolkit.Maui.Markup

This means that we would publish two NuGet Packages from this repo:

  • CommunityToolkit.Maui
  • CommunityToolkit.Maui.Core

Here's my recommendation:

  • Create a new CSPROJ, CommunityToolkit.Maui.Core
    • Move all Interfaces and common code to this new CSPROJ
    • Create a new NuGet Package: CommunityToolkit.Maui.Core
  • Keep the current name of CommunityToolkit.Maui instead of renaming it to CommunityToolkit.Maui.Controls
    • Renaming the project to CommunityToolkit.Maui.Controls requires renaming the NuGet package
    • The CommunityToolkit.Maui NuGet Package will then have a dependency on the CommunityToolkit.Maui.Core NuGet Package
    • In the dotnet/maui repo, [InternalsVisibleTo] is already enabled for CommunityToolkit.Maui
      • Right now, we are experiencing breaking changes in Snackbar.android.cs because it cannot access internals
  • Do not follow the dotnet/maui folder structure
    • I'd like to avoid their confusing + unnecessary nesting e.g. src/core/src/core...

Recommended Architecture

CommunityToolkit/Maui
└───src
│   │   | CommunityToolkit.Maui.sln
│   │
│   └───CommunityToolkit.Maui
│       │   CommunityToolkit.Maui.csproj
│   └───CommunityToolkit.Core
│       |   CommunityToolkit.Maui.Core.csproj
│   └───CommunityToolkit.UnitTests
│       |   CommunityToolkit.Maui.UnitTests.csproj
└───samples
    │   CommunityToolkit.Maui.Samples.csproj

@TheCodeTraveler
Copy link
Collaborator

TheCodeTraveler commented Dec 9, 2021

Will we actually share code between other Toolkits?

Looking at the Comet source code, I'm not sure how much code we would be able to share between CommunityToolkit.Maui and CommunityToolkit.Comet.

For example, Comet doesn't use Converters (that's mostly an MVVM/XAML thing), Comet doesn't have Behaviors, Comet uses its own unique Animation implementation and Comet creates its own Controls (e.g. it provides its own implements of IButton, IImage, etc.).

If there isn't a future where new toolkits would leverage CommunityToolkit.Maui.Core, does it make sense to separate it into its own CSPROJ + its own NuGet Package?

@VladislavAntonyuk
Copy link
Collaborator

I suggest to keep in CommunityToolkit.Maui.Core only interfaces and base classes, so it will be platform agnostic (net6.0).
We put ISnackbar, BaseAnimation, IValidator etc here.

Then CommunityToolkit.Maui contains animations, converters, validators, but not Controls (Views). So it can be net6.0 as well. This library doesn't need UseCommunityMauiToolkit().

Finally CommunityToolkit.Maui.Controls implementation contains only controls and depends on Platforms.

@pictos
Copy link
Member Author

pictos commented Dec 9, 2021

@brminnick I agree with everything in your first comment, I'll apply those changes and push them to my working branch.

About the Comet, as you can see here controls are generated using Source Generator, the input needed is the interfaces, so Comet will need just to reference the CommunityToolkit.Maui.Core. About other features like Behaviors, Converters, etc we don't need to share with Comet if there's no support for it.

I'll add @Clancey to add anything else.

Expanding a little bit, I believe that any UI framework built on top of .NET MAUI can reference CommunityToolkit.Maui.Core.

@pictos
Copy link
Member Author

pictos commented Dec 9, 2021

@VladislavAntonyuk

I'm not sure if makes sense to restrict one package to be platform agnostic, as you can see here this class lives in Core and has platform code. I would say that we can make it more general possible and use platform-specific code, what do you think?

Are you suggesting to have three csprojs?
How that will be in terms of NuGet, will we release one NuGet per package, or we can merge Core and Controls into a single package?
Why split Views and other features (Animation, behaviors, etc)?

@VladislavAntonyuk
Copy link
Collaborator

VladislavAntonyuk commented Dec 9, 2021

@pictos something like this:
image

the idea is to split platforms code from net6.0. for example, validators don't require Android API, so they should not depend on it. from the screenshot, you can see that Controls lib doesn't target net6.0. it means we do not allow people to add this lib to WPF for example. so we do not need to throw exception Platform is not supported.

so if I would like to use converters in the WPF application, I can simply reference the project. and use it: var converter = new MyConverter().Convert("value", MyType, MyParameter, MyCulture);

As for release, yes, we will have 3 packages. but it doesn't mean we need more effort to support them all. most likely they will be released in the same time in 1 pipeline

@saint4eva
Copy link

Will we actually share code between other Toolkits?

Looking at the Comet source code, I'm not sure how much code we would be able to share between CommunityToolkit.Maui and CommunityToolkit.Comet.

For example, Comet doesn't use Converters (that's mostly an MVVM/XAML thing), Comet doesn't have Behaviors, Comet uses its own unique Animation implementation and Comet creates its own Controls (e.g. it provides its own implements of IButton, IImage, etc.).

If there isn't a future where new toolkits would leverage CommunityToolkit.Maui.Core, does it make sense to separate it into its own CSPROJ + its own NuGet Package?

CommunityToolkit.Maui.Comet - Comet app model
CommunityToolkit.Maui.Blazor - Blazor app model
CommunityToolkit.Maui.Controls - Xaml app model
CommunityToolkit.Maui.Fabulous - Fabulous app model

@pictos
Copy link
Member Author

pictos commented Dec 10, 2021

@VladislavAntonyuk I'm sorry for the late reply, yesterday was a rush day.

So I can see what you mean, and I would say that we don't need more than 2 projects. From the MS.SDK project style all platforms targets have the NET 6 reference, so if we don't want to throw NotImplementedException we can do that by not adding the NET 6 as a TFM, in other words, that project can be referenced, just by the platforms that we add as TFM.

Also, since this project targets specific platforms (mobile and desktop) it's fine, IMHO, that we have platform code on Core and CommunityToolkit projects.

@VladislavAntonyuk
Copy link
Collaborator

@pictos no problem.

if we do not include net6.0 in target frameworks we will receive an issue similar to this: xamarin/XamarinCommunityToolkit#1645

Maybe we can rename Core to Shared in that case we receive

  • CommunityToolkit.Maui.Shared (it includes all control interfaces and platform-agnostic code like validators and converters)
  • CommunityToolkit.Maui (if we want to keep the original name) or CommunityToolkit.Maui.Controls (which doesn't include net6.0 and contains all platform-specific code)

@pictos
Copy link
Member Author

pictos commented Dec 10, 2021

@VladislavAntonyuk this makes sense to me. Like, if someone references the Controls project it's because that person wants to use platform-specific code. I know that we don't need to be 100% equals to MAUI, but I'll take a look to see if I can discover why Behavior is on Controls project instead of Core

@pictos
Copy link
Member Author

pictos commented Dec 10, 2021

@brminnick @VladislavAntonyuk just pushed the changes based on our conversation. I didn't rename the Core to Shared yet

@TheCodeTraveler
Copy link
Collaborator

TheCodeTraveler commented Dec 11, 2021

@pictos Could you please provide a definition for CommunityToolkit.Maui.Core?

Providing a drawing/diagram of the architecture would best help to clarify your vision.

I'm still having trouble understanding what belongs into CommunityToolkit.Maui.Core and what remains in CommunityToolkit.Maui, and reading through the discussions in the PR Review, it's clear that we have not properly defined it yet.

Here's some questions I have for CommunityToolkit.Maui.Core:

  • Will it contain code from the Microsoft.Maui namespace?
    • I vote that it should
  • Will it contain code from the Microsoft.Maui.Controls namespace?
    • I vote that it should not
  • Will it contain code from Microsoft.Maui.Graphics namespace?
    • I vote that it should be allowed, but not required
  • Will it contain platform-specific Handlers?
    • I vote that it should contain all Handlers
    • For example, if ICalendar and its platform-specific Handlers are defined in CommunityToolkit.Maui.Core, separate Toolkits, like CommunityToolkit.Comet, just need to create the MVU implementation of ICalendar and the platform-specific implementation will just work ™️

Recommended Architecture

Here's my recommended decision chart:

`CommunityToolkit.Maui.Core`  <-  [No] <-  "Does it use `Microsoft.Maui.Controls`?" -> [Yes] -> `CommunityToolkit.Maui`

Using this, we arrive at this architecture:

CommunityToolkit.Maui
└── Alerts (`Snackbar` requires `Microsoft.Maui.Controls.VisualElement`)
└── Behaviors (Requires `Microsoft.Maui.Controls.Behavior<T>`)
└── Converters (Requires `Microsoft.Maui.Controls.IValueConverter`)
└── Extensions (Certain extensions, like `ColorAnimationExtensions` require `Microsoft.Maui.Controls.VisualElement`)
└── Views (Any view that requires `Microsoft.Maui.Controls`, e.g. `Popup.shared.cs`)

CommunityToolkit.Maui.Core
└── Interfaces (E.g. `ISnackbar`)
└── Extensions (Any extension that doesn't require `Microsoft.Maui.Controls`, like `ColorConversionExtensions`)
└── Handlers (All Handlers)
└── Views (Any custom platform-specific views that don't use `Microsoft.Maui.Controls`, e.g. `Popup.ios.macos.cs`, `PopupView.ios.macos.cs`, `Snackbar.ios.macos.cs`, etc)

image

@TheCodeTraveler TheCodeTraveler added in discussion and removed approved This Proposal has been approved and is ready to be added to the Toolkit labels Dec 11, 2021
@TheCodeTraveler
Copy link
Collaborator

Removing the approved tag until we define the architecture and add it to the Detailed Design section of the Proposal

@pictos
Copy link
Member Author

pictos commented Dec 11, 2021

@brminnick and @VladislavAntonyuk just a heads up that I updated the description with a more complete information

@TheCodeTraveler
Copy link
Collaborator

TheCodeTraveler commented Dec 11, 2021

Thanks @pictos!

The core library will need to be called CommunityToolkit.Maui.Core for a couple reasons:

  • The CommunityToolkit org already has a CommunityToolkit.Common NuGet Package
    • Creating CommunityToolkit.Core would cause confusion between the two packages
  • It relies on Microsoft.Maui
    • Including Maui in CommunityToolkit.Maui.Core easily notifies developers about the dependence on .NET MAUI

It sounds like we're almost aligned on the architecture. , with the exception of Handlers.

I recommend keeping Handlers should be in CommunityToolkit.Maui.Core for a few reasons:
- The .NET MAUI team has their Handlers in the Core library
- Handler code can be reused by other toolkits, like CommunityToolkit.Comet

(Never mind - I misread the Handlers section in Detailed Design)

👇 Do you concur with this flow diagram? If so, we should add it to the README and the Contribution Guidelines (FYI - @bijington for when you update them).

image

@pictos
Copy link
Member Author

pictos commented Dec 11, 2021

@brminnick I agree with the image.

@Clancey
Copy link

Clancey commented Dec 11, 2021

The talk, on how comet would work is correct. You would be able to re-use anything in CommunityToolkit.Maui.Core for Comet. So all the handlers should work. If the handlers use interfaces, you can auto generate the comet code!

https://github.com/dotnet/Comet/blob/dev/src/Comet/Controls/ControlsGenerator.cs

This is how comet actually adds most of its controls!

@TheCodeTraveler
Copy link
Collaborator

Right on! @pictos - When you get a chance, update the Detailed Design to use CommunityToolkit.Maui.Core, and then we'll do a quick vote on it with the team to approve it 🙌

Normally, we'd wait to vote until the next standup, but since this is a blocking PR, we can have everyone comment on this Proposal with "Approved" then we'll add the approved tag 👍

@pictos
Copy link
Member Author

pictos commented Dec 11, 2021

@brminnick I will update everything until tomorrow. Anyway I'll ping the team on discord when I do it.

@TheCodeTraveler TheCodeTraveler changed the title [Enhancement] Project reorganization [Proposal] Project Reorganization Dec 12, 2021
@TheCodeTraveler TheCodeTraveler added the champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature label Dec 12, 2021
@TheCodeTraveler
Copy link
Collaborator

I vote to Approve ✅

1 similar comment
@pictos
Copy link
Member Author

pictos commented Dec 12, 2021

I vote to Approve ✅

@VladislavAntonyuk
Copy link
Collaborator

Approve

@ghost ghost added approved This Proposal has been approved and is ready to be added to the Toolkit help wanted This proposal has been approved and is ready to be implemented labels Dec 12, 2021
@TheCodeTraveler TheCodeTraveler removed help wanted This proposal has been approved and is ready to be implemented in discussion labels Dec 12, 2021
@bijington
Copy link
Contributor

I vote to Approve

@jfversluis
Copy link
Member

Lets approve this one!

@ghost
Copy link

ghost commented Apr 15, 2022

Reopening Proposal.

Only Proposals moved to the Closed Project Column and Completed Project Column can be closed.

@ghost ghost reopened this Apr 15, 2022
@ghost ghost closed this as completed Apr 15, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 23, 2024
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved This Proposal has been approved and is ready to be added to the Toolkit champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature documentation approved ♻ housekeeping This issue/PR is related to internal stuff.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants