-
Notifications
You must be signed in to change notification settings - Fork 361
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
Update Microcharts to .NET MAUI 6 #325
Conversation
Interestingly, on MacCat/iOS when I run the app directly without the debugger it initially launches fine and is interactive, but then later crashes. |
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. |
57bd104
to
a57027f
Compare
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. |
@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. |
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? |
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! |
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. |
Look good to me. I think we can merge this at a starting point to the MAUI compatibility. Thanks @Eilon for this work ! |
This is brilliant. I tested it on Android and with this our ported MAUI app achieves feature parity with the original Xamarin Forms app. EDIT: Though it seems that setting the |
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. |
There was a problem hiding this 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.
Hi @Eilon, Any chance Microcharts will work with Blazor via SkiaSharp Blazor package ? That will be great especially for MAUI Blazor. |
No, that's a web technology and this library is meant for mobile app usage. |
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 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. 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: 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. |
Could someone please answer these questions: |
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. |
@eman1986 Thank you very much. When it becomes available, I will start testing it and post if I find an issue. |
Sources/Microcharts.Samples.Forms/Microcharts.Samples.Forms/Microcharts.Samples.Forms.csproj
Show resolved
Hide resolved
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! |
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! |
@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. |
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!) |
@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. |
I appreciate all your work, I'll take a look at this tomorrow, |
@eman1986 that sounds great! |
Is there something needed to get this PR to be merged? Something I could help you with? |
Sorry, I was offline for a bit dealing with a family emergency. I'll give this a look over tomorrow. |
I'm sorry to hear that |
@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. |
Thankfully things are improving so I can get back to things. I'll try to take a peek at this this weekend. |
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.) |
I'll need to renew the secret, I'm pretty sure it has expired by now |
I updated the deploy secret |
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? |
@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). |
Updating #280 to use .NET MAUI 6 RTM.
Status: