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 Microcharts to .NET MAUI 6 #325

Merged
merged 39 commits into from
Apr 16, 2023

Conversation

Eilon
Copy link
Collaborator

@Eilon Eilon commented Jan 25, 2023

Updating #280 to use .NET MAUI 6 RTM.

Status:

  • WinUI: Works
  • Android: Works
  • MacCatalyst: Sample app launches and shows charts, but immediately hangs
  • iOS: Sample app launches and shows charts, but immediately hangs

@Eilon Eilon mentioned this pull request Jan 25, 2023
8 tasks
@Eilon
Copy link
Collaborator Author

Eilon commented Jan 25, 2023

Interestingly, on MacCat/iOS when I run the app directly without the debugger it initially launches fine and is interactive, but then later crashes.

@Eilon
Copy link
Collaborator Author

Eilon commented Jan 25, 2023

And now after I ran it a few more times, it seems to work without crashing. Not sure at all what's happening. It's also not clear if it's an issue with the core Microcharts, the MAUI layer of Microcharts, MAUI itself, or an artifact of the sample app itself.

@Eilon Eilon force-pushed the eilon/update-pr280-maui-net6 branch from 57bd104 to a57027f Compare January 25, 2023 19:17
@Eilon
Copy link
Collaborator Author

Eilon commented Jan 25, 2023

I rebased the PR branch on top of latest main so in theory it's mergeable, but I'm sure it's not ready yet. I think several versions of things need to be updated, for example.

@Eilon
Copy link
Collaborator Author

Eilon commented Jan 25, 2023

@Seuleuzeuh / @eman1986 - if either or both of you would like to chat more about this please let me know and we can arrange a time for it. I'm located in US West Coast (GMT-8).

In the meantime I'll keep working on this branch/PR.

@Eilon
Copy link
Collaborator Author

Eilon commented Jan 25, 2023

Alright I think this is in at least decent shape, so I'd love for another person to try it out and see if everything works for them. I'm still not sure about the seemingly random hanging issues on iOS/MacCat.

Is there anything else required for this PR to get it merged in?

@Eilon Eilon mentioned this pull request Jan 25, 2023
@Eilon Eilon changed the title Eilon/update pr280 maui net6 Update Microcharts to .NET MAUI 6 Jan 25, 2023
@Seuleuzeuh
Copy link
Contributor

I dont' have much time, i'll try to look at it tomorow morning (GMT +1) before work.

For the Nuget generation and publish part, we'll wait for @eman1986.

@Eilon
Copy link
Collaborator Author

Eilon commented Jan 25, 2023

I dont' have much time, i'll try to look at it tomorow morning (GMT +1) before work.

For the Nuget generation and publish part, we'll wait for @eman1986.

Sounds great!

BTW I'm an engineer on the .NET team at Microsoft and we've heard that this project is requested by some people to be available in .NET MAUI, so I figured I'd try to help out with this project!

@eman1986
Copy link
Member

I appreciate that 😊

@Eilon
Copy link
Collaborator Author

Eilon commented Jan 25, 2023

I appreciate that 😊

My pleasure!

Please let me know what I can do to help so we could hopefully get an update MAUI-compatible package on NuGet.org.

@Seuleuzeuh
Copy link
Contributor

Look good to me.
I've not reproduce the hanging in the iOS version (i'me unable to try it on my Mac). I've to delete the dotnet_bot image to make the sample run.
The sample contains some visual issue, but not linked to the Chart part so it's not a real problem.
All the Chart loads correctly, and without problem according to all the parameters.

I think we can merge this at a starting point to the MAUI compatibility.

Thanks @Eilon for this work !

@Ghostbird
Copy link

Ghostbird commented Jan 27, 2023

This is brilliant. I tested it on Android and with this our ported MAUI app achieves feature parity with the original Xamarin Forms app.
It's suprisingly easy to build Microcharts.MAUI for .NET 7.0 + Android API 33 on Linux that's what I used to test this.

EDIT: Though it seems that setting the LabelTextSize doesn't change the text size, at least on the DonutChart.

@eman1986
Copy link
Member

Thanks for your hard work @Eilon I'll take a look at this later today (it's fairly late right now) getting this to be maui ready will be really exciting.

@Eilon
Copy link
Collaborator Author

Eilon commented Jan 30, 2023

Thanks for your hard work @Eilon I'll take a look at this later today (it's fairly late right now) getting this to be maui ready will be really exciting.

Fantastic! Please let me know if you have any questions. Or, I'd be happy to jump on a phone call if it would help with anything.

Copy link

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

Awesome to see stand-alone support for WinUI 3 being added as part of this as well! 🦙❤

Adding a few comments from a quick pass.

NuGet/Microcharts.Android.nuspec Outdated Show resolved Hide resolved
NuGet/Microcharts.WinUI.nuspec Outdated Show resolved Hide resolved
NuGet/Microcharts.WinUI.nuspec Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Sources/Microcharts.WinUI/ChartView.cs Outdated Show resolved Hide resolved
@Pinox
Copy link

Pinox commented Jan 30, 2023

Hi @Eilon,

Any chance Microcharts will work with Blazor via SkiaSharp Blazor package ? That will be great especially for MAUI Blazor.

https://www.nuget.org/packages/SkiaSharp.Views.Blazor

@eman1986
Copy link
Member

No, that's a web technology and this library is meant for mobile app usage.

@Pinox
Copy link

Pinox commented Jan 31, 2023

Hi @eman1986 ,

Everything in the image below is MAUI Blazor and I would dare to say the mobile app is probably the most used. I'm not referring to a pure web scenario in any way

image

The Mobile app still uses MAUI to compile to a native MAUI app for Android and iOS. Only difference here is you will throw in the Skiasharp nuget above and add BlazorWebView control and now a UI rendered through the webview only for the specific chart and not per se for the rest of the application. They also call this hybrid MAUI where you can mix native + webUI in the same app. All the code behind will be exactly the same as what will be in Android + iOS.

MAUI Hybrid =>
image

So you can picture the "WEB UI" above as a Microchart chart within a native app.

The code for the webview is as simple as this:

image

Reason for my request it will be great to use this in MAUI Blazor but also adds an alternative (backup) for the main Android + iOS branch functionality. So if some functionality in the main Android branch is not working but it's working through the Webview control then it easy to switch and incorporate into your native android or iOS mobile app.

@upswing1
Copy link

Could someone please answer these questions:
How far away are we from merging this PR and creating a preview Nuget package?"
Are there any plans for .NET 7?
Is this PR utilizing MAUI.Graphics?

@eman1986
Copy link
Member

I can't answer the last bit but I'll look at this tonight and I'll get a new preview release out, I was holding out for RTL support before making a 1.0 release but I think we'll just have to wait for someone who really wants it to make a PR for it.

@upswing1
Copy link

@eman1986 Thank you very much. When it becomes available, I will start testing it and post if I find an issue.

@eman1986
Copy link
Member

eman1986 commented Feb 1, 2023

there's a few changes needed, once those are addressed I will merge this in.

@Eilon
Copy link
Collaborator Author

Eilon commented Feb 1, 2023

there's a few changes needed, once those are addressed I will merge this in.

I pushed an update that I think addresses all the earlier comments. Please let me know what you think and if there's anything I missed or needs more work. Thanks!

@Eilon
Copy link
Collaborator Author

Eilon commented Mar 10, 2023

Alright, it only took like a hundred commits, but I have the PR build working fine on my fork, just to test that it's working. If that gets merged in here and enabled, then every PR will get some basic verification that at least it compiles!

@Eilon
Copy link
Collaborator Author

Eilon commented Mar 10, 2023

@eman1986 - if you can do a final review of this and merge, then we can get to the next step, which is to build & publish NuGet packages to nuget.org. I see there are already some YML actions for that in this repo, so it's hopefully quite easy, as long as you still have the secrets for it set up.

@Eilon
Copy link
Collaborator Author

Eilon commented Mar 10, 2023

You can see the PR action that I added by looking at my test PR in my fork: https://github.com/Eilon/Microcharts/pull/1/checks (it's still running one of the legs as of now, but it should succeed jinx!)

@Eilon
Copy link
Collaborator Author

Eilon commented Mar 17, 2023

@eman1986 just checking in to see if you have a sense of when you could take a look at this. Or if you want to optimistically merge it in 😄 As before, I'm happy to jump on a call with you if that makes sense.

@eman1986
Copy link
Member

I appreciate all your work, I'll take a look at this tomorrow,

@Eilon
Copy link
Collaborator Author

Eilon commented Mar 20, 2023

@eman1986 that sounds great!

@JelleDamen
Copy link

Is there something needed to get this PR to be merged? Something I could help you with?

@eman1986
Copy link
Member

Sorry, I was offline for a bit dealing with a family emergency. I'll give this a look over tomorrow.

@JelleDamen
Copy link

I'm sorry to hear that ☹️

@Eilon
Copy link
Collaborator Author

Eilon commented Apr 14, 2023

@eman1986 no worries, that sounds a lot more important than charts!

As before, I'm happy to work with you on any feedback or questions you may have. I'm happy to chat here, on the phone, or even a Teams call if you like.

@eman1986
Copy link
Member

Thankfully things are improving so I can get back to things. I'll try to take a peek at this this weekend.

@eman1986 eman1986 merged commit a6cbfd0 into microcharts-dotnet:main Apr 16, 2023
@Eilon
Copy link
Collaborator Author

Eilon commented Apr 16, 2023

Thank you @eman1986 ! I think there's probably a follow-up PR that I'll work on to make sure that the CI build can build and push the new set of packages (not just MAUI, but a few other new packages from earlier PRs).

@eman1986 can you confirm that you are still able to push the NuGet packages via the GitHub actions? (You might need to update the secret API key if it has expired. I've recently gone through a similar process and can assist with that as well if you like.)

@eman1986
Copy link
Member

I'll need to renew the secret, I'm pretty sure it has expired by now

@eman1986
Copy link
Member

I updated the deploy secret

@JelleDamen
Copy link

@eman1986 - if you can do a final review of this and merge, then we can get to the next step, which is to build & publish NuGet packages to nuget.org. I see there are already some YML actions for that in this repo, so it's hopefully quite easy, as long as you still have the secrets for it set up.

Great work on the PR, I would love to continue use this library. Is there any planning on when this could be build/deployed for maui?

@Eilon
Copy link
Collaborator Author

Eilon commented Apr 29, 2023

@JelleDamen - I have a PR to this repo to update the build scripts so that the packages can be published to NuGet.org: #328

You could build the packages locally in the meantime (or clone and reference the CSPROJ file from your own project), though of course that's not nearly as easy (but only like 30min work though).

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.

9 participants