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

Update MBB to use MAUI #409

Merged
merged 69 commits into from
Jun 9, 2022
Merged

Update MBB to use MAUI #409

merged 69 commits into from
Jun 9, 2022

Conversation

Dreamescaper
Copy link
Contributor

@Dreamescaper Dreamescaper commented Sep 18, 2021

#352

This PR is based on the other PRs, so they should be reviewed first: #388, #395, #345, #346, #343, #369, #392, #425, #419.

Now, I'm not sure how this PR should be handled. Should it go to some separate branch? Maybe even other repo?

  • Update ComponentWrapperGenerator to generate MAUI wrapper
  • Update MobileBlazorBindings to use MAUI
  • Rework Xaminals sample to use single project, MAUI, and updated MBB
  • Test it on Windows
  • Test it on Mac
  • Test it on iOS
  • Rework other samples
  • Update templates
  • Update the project to use MAUI's BlazorWebView

(I've logged a couple of MAUI bugs when working on this one:
dotnet/maui#2623, dotnet/maui#2601, dotnet/maui#2600, dotnet/maui#2591, dotnet/maui#2958, dotnet/maui#4729, dotnet/maui#4765
)

@Dreamescaper
Copy link
Contributor Author

recording

aBYT8qIQCx

@Dreamescaper Dreamescaper marked this pull request as draft September 18, 2021 19:13
@Eilon
Copy link
Member

Eilon commented Sep 21, 2021

Oh BOY!

@Dreamescaper
Copy link
Contributor Author

Hi @Eilon!
MAUI has reached RC phase, so I'd like to ping about the status of this PR and the project overall.
Are there any plans to keep this repo moving forward?
I really like this library, I use it for my projects, and I plan to work on it further in the nearby future.

If it can't be supported here, I'm thinking about forking it, and continuing working on that separately (at least until it is supported).
Any other options (like moving it to CommunityToolkit or smth like that, not sure) ?

@Dreamescaper
Copy link
Contributor Author

I have forked this repository and published to nuget (renamed to BlazorBindings.Maui).
You can read more here.
If at any point of time Microsoft developers decide to push this repository moving forward, I'll gladly contribute all of my changes here.

@Eilon
Copy link
Member

Eilon commented Jun 8, 2022

I'm taking a crack at this 😁

image

@Dreamescaper
Copy link
Contributor Author

@Eilon Yeah, just run "git clean -fdx" with your VS closed. It doesn't like remains of pre-MAUI samples parts.

@Eilon
Copy link
Member

Eilon commented Jun 8, 2022

Ah, git clean -xdf and a rebuild got me past that! My CPU is still on fire, though!

========== Build: 16 succeeded, 0 failed, 0 up-to-date, 0 skipped ==========

image

@Eilon
Copy link
Member

Eilon commented Jun 8, 2022

Well, so far it's working quite fine! I think I hit a few issues but I can't tell if it's an MBB issue or a MAUI issue.

@Eilon
Copy link
Member

Eilon commented Jun 8, 2022

I think the only real issue I've hit so far is this exception: https://github.com/Dreamescaper/BlazorBindings.Maui/blob/maui/src/Microsoft.MobileBlazorBindings/Elements/Handlers/ModalContainerHandler.cs#L84-L85

To repro:

  1. Run MBB ToDo sample on Windows/Android (didn't try Apple platforms yet)
  2. Make sure there's at least one item in the list
  3. Click the "..." button next to one of the todo items to launch the "modal" dialog
  4. Press either Save or Cancel (anything to dismiss the "modal"

Result:

{System.NotImplementedException: The method or operation is not implemented.
   at Microsoft.MobileBlazorBindings.Elements.Handlers.ModalContainerHandler.RemoveChild(Element child) in C:\GitHub\MobileBlazorBindings\src\Microsoft.MobileBlazorBindings\Elements\Handlers\ModalContainerHandler.cs:line 85
   at Microsoft.MobileBlazorBindings.MobileBlazorBindingsElementManager.RemoveChildElement(IMauiElementHandler parentHandler, IMauiElementHandler childHandler) in C:\GitHub\MobileBlazorBindings\src\Microsoft.MobileBlazorBindings\MobileBlazorBindingsElementManager.cs:line 79
   at Microsoft.MobileBlazorBindings.Core.ElementManager`1[[Microsoft.MobileBlazorBindings.IMauiElementHandler, Microsoft.MobileBlazorBindings, Version=0.6.0.0, Culture=neutral, PublicKeyToken=null]].RemoveChildElement(IElementHandler parentHandler, IElementHandler childHandler) in C:\GitHub\MobileBlazorBindings\src\Microsoft.MobileBlazorBindings.Core\ElementManagerOfElementType.cs:line 45
   at Microsoft.MobileBlazorBindings.Core.NativeComponentAdapter.RemoveSelfAndDescendants() in C:\GitHub\MobileBlazorBindings\src\Microsoft.MobileBlazorBindings.Core\NativeComponentAdapter.cs:line 134
   at Microsoft.MobileBlazorBindings.Core.NativeComponentAdapter.RemoveSelfAndDescendants() in C:\GitHub\MobileBlazorBindings\src\Microsoft.MobileBlazorBindings.Core\NativeComponentAdapter.cs:line 141
   at Microsoft.MobileBlazorBindings.Core.NativeComponentAdapter.ApplyRemoveFrame(Int32 siblingIndex) in C:\GitHub\MobileBlazorBindings\src\Microsoft.MobileBlazorBindings.Core\NativeComponentAdapter.cs:line 125
   at Microsoft.MobileBlazorBindings.Core.NativeComponentAdapter.ApplyEdits(Int32 componentId, ArrayBuilderSegment`1 edits, ArrayRange`1 referenceFrames, RenderBatch batch, HashSet`1 processedComponentIds) in C:\GitHub\MobileBlazorBindings\src\Microsoft.MobileBlazorBindings.Core\NativeComponentAdapter.cs:line 66
   at Microsoft.MobileBlazorBindings.Core.NativeComponentRenderer.UpdateDisplayAsync(RenderBatch& renderBatch) in C:\GitHub\MobileBlazorBindings\src\Microsoft.MobileBlazorBindings.Core\NativeComponentRenderer.cs:line 102
   at Microsoft.AspNetCore.Components.RenderTree.Renderer.ProcessRenderQueue()}
    base: {System.SystemException}

Expected:
No exception.

To be honest, everything about how modals are implemented in MBB is quite hacky. I'm sure there's some good code in there, but I don't know where 😄 I wouldn't have this prevent me from merging the PR, but rather I'd log it as a separate issue to investigate.

@Eilon
Copy link
Member

Eilon commented Jun 8, 2022

BTW this code didn't change as part of this PR, so it might be caused by some other change in this PR, or a change in .NET MAUI.

@Dreamescaper
Copy link
Contributor Author

Dreamescaper commented Jun 8, 2022

I think it is fine simply to remove throwing that exception, make it no-op. But yeah, I planned to rework dealing with modals anyway (or rather clean some parts that I already have). I've drafted a small proposal here: #421 .

@Eilon
Copy link
Member

Eilon commented Jun 9, 2022

Yeah without the exception it seems to just work fine 😄

@Eilon
Copy link
Member

Eilon commented Jun 9, 2022

Aside from iOS/macOS testing, is there anything else you think needs to be done before merging? I've reviewed a lot of the code so far and I think it's good, but I will finish reviewing first anyway.

@Dreamescaper
Copy link
Contributor Author

I've tested iOS, it seemed to be fine (not catalyst though).

I think it's fine at this point. There are some samples which could be migrated as well, but i would handle that separately if needed.

@Eilon
Copy link
Member

Eilon commented Jun 9, 2022

Another thing I noticed is that the Component Generator doesn't copy doc comments anymore. We'll definitely want to bring that back, but it might need to work a bit differently due to how the MAUI packages are created (but as of MAUI GA they XML files at least exist).

@Eilon Eilon merged commit 4a1f867 into dotnet:main Jun 9, 2022
@Eilon
Copy link
Member

Eilon commented Jun 9, 2022

Well, it's merged! 🥳

@Eilon Eilon mentioned this pull request Jun 9, 2022
@dcuccia
Copy link

dcuccia commented Jun 9, 2022

Wonderful, can't wait to try it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants