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

Adding Uno v3 support #3898

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

nickrandolph
Copy link
Contributor

@nickrandolph nickrandolph commented Sep 2, 2020

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Feature - Adds support for Uno

⤵️ What is the current behavior?

No support for Uno

🆕 What is the new behavior (if this is a feature change)?

Additional libraries MvvmCross.Uno and MvvmCross.Uno.Plugins to support Uno

💥 Does this PR introduce a breaking change?

No

🐛 Recommendations for testing

Run the sample Uno projects

📝 Links to relevant issues/docs

Fixes #3897

🤔 Checklist before submitting

@nickrandolph
Copy link
Contributor Author

I've created a new PR that cleans up the commit history for adding Uno along with rebasing off current develop. This deprecates the older PR that I've closed (#3495)

@MartinZikmund MartinZikmund mentioned this pull request Sep 2, 2020
8 tasks
@nickrandolph
Copy link
Contributor Author

@Cheesebaron are you able to assist with getting the Sonar check to pass - seems to have an authentication error
image

@Cheesebaron
Copy link
Member

Sonar is a separate check now, not part of the CI build. Make sure to rebase/merge develop

@nickrandolph
Copy link
Contributor Author

@Cheesebaron we rebased yesterday so branch should be up to date. I'm still investigating the build fails but the Sonar fail appears to be authentication related (see image I attached). Is there anything I need to do to fix that?

@nickrandolph
Copy link
Contributor Author

Woot! build success. @Cheesebaron Sonar still failing though :-(

@Cheesebaron
Copy link
Member

Yeah Sonar fails on external branches right now. Don't worry about that.

@Cheesebaron
Copy link
Member

Sorry for the delay @nickrandolph. This is pretty cool. We probably need to work a bit on the Presenter stuff, because some of the back navigation doesn't work. However, this should probably not prevent this work from getting in.

Some observations:

  • pressing back on android using the hardware back button does nothing when you are on the first view.
  • pressing close on the "Second Child View" does nothing on any platform tested
  • pressing "Second Child" in the Child View on WASM does nothing

Tested Android, UWP and WASM. All of them seem to work fairly similarly. Will test iOS and macOS tomorrow-ish.

@nickrandolph
Copy link
Contributor Author

@Cheesebaron thanks for testing this out. It might be good to get this PR in and to just mark the Uno library as prerelease. Currently we're not doing anything special for Uno, so any navigation that doesn't work will most likely be a scenario we need to add custom logic for. Let me know how you'd like to proceed.

@Cheesebaron
Copy link
Member

@nickrandolph sure. Lets get this in. Then we can address the issues with presentation and navigation in separate issues.

Can you mark it ready for review?

@Cheesebaron Cheesebaron self-assigned this Sep 22, 2020
@Cheesebaron Cheesebaron added the t/feature Feature request type label Sep 22, 2020
@nickrandolph nickrandolph marked this pull request as ready for review September 23, 2020 02:43
@nickrandolph nickrandolph requested a review from a team September 23, 2020 02:43
@nickrandolph
Copy link
Contributor Author

@Cheesebaron done - ping me if you need any changes as part of the review

@Cheesebaron
Copy link
Member

Yeah, at least the conficts 😄

@nickrandolph
Copy link
Contributor Author

@Cheesebaron argh, where'd that conflict appear from - I'll take a look when I get a chance.

@amirvenus
Copy link

@Cheesebaron argh, where'd that conflict appear from - I'll take a look when I get a chance.

Would be great to have it merged at some point Thanks!

@Cheesebaron
Copy link
Member

I am happy to take the contribution. As is now, it can't be merged as it doesn't build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t/feature Feature request type
Development

Successfully merging this pull request may close these issues.

Uno Platform support
3 participants