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

122 UniformGrid #139

Merged
merged 18 commits into from
Dec 10, 2021
Merged

122 UniformGrid #139

merged 18 commits into from
Dec 10, 2021

Conversation

VladislavAntonyuk
Copy link
Collaborator

@VladislavAntonyuk VladislavAntonyuk commented Oct 12, 2021

Fixes

#122

Description

Migrate UniformGrid to MCT
image

@VladislavAntonyuk
Copy link
Collaborator Author

I added samples in this PR (#5)
So decided to keep it in 1 review and add samples for UniformGrid later

@pictos
Copy link
Member

pictos commented Oct 12, 2021

Hey, @VladislavAntonyuk hope that you're doing fine. I have a couple of comments on this PR, one brings up our new contribution flow and the other about the implementation.

Not sure you saw our latest Community Stand-up where @brminnick showed our contribution process. The contribution process changed from the old XCT.

The first step is to ask in the issue to work on it and wait for us to confirm that you can work on that issue, this will help us to keep everything on track.

About the code implementation, the Layout API was changed in the .NET MAUI, this one is the legacy API with that I believe that we want to migrate this layout to the newest API.

Happy to see that we will have your contributions here as well <3

@brminnick brminnick added the do not merge Do not merge this PR label Oct 12, 2021
@brminnick
Copy link
Collaborator

Hi @VladislavAntonyuk! Echoing Pedro, thanks again for your hard work!

I've temporarily added the do not merge tag here to pause and catch up on the process before we review/merge the PR.

Here's the steps for us to catch up on the paper work:

  1. Please leave a comment on [Proposal] UniformItemsLayout #122
  2. The Proposal Champion (in this case, @jsuarezruiz) will then remove the help wanted tag and assign it to you

This all ensures that we don't have multiple people working on the same Proposal.

@VladislavAntonyuk VladislavAntonyuk changed the title 122 UniformGrid 122 UniformGrid, Fix for .NET RC2 Oct 12, 2021
@VladislavAntonyuk
Copy link
Collaborator Author

@pictos updated the code to remove the usage of the Compatibility namespace.
@pictos , @brminnick thank you for providing the link for the new flow.

Also added the fix for the new .net 6-rc2

@VladislavAntonyuk VladislavAntonyuk changed the title 122 UniformGrid, Fix for .NET RC2 122 UniformGrid, Fix for .NET 6 RC2 Oct 12, 2021
@pictos
Copy link
Member

pictos commented Oct 12, 2021

@VladislavAntonyuk Thanks for that, I'll review it asap

@VladislavAntonyuk VladislavAntonyuk changed the title 122 UniformGrid, Fix for .NET 6 RC2 122 UniformGrid Oct 16, 2021
@VladislavAntonyuk
Copy link
Collaborator Author

@brminnick the implementation is updated to remove Xamarin compatibility namespace. is DoNotMerge label still valid? (I just don't like this name 😄 )

@brminnick
Copy link
Collaborator

@brminnick the implementation is updated to remove Xamarin compatibility namespace. is DoNotMerge label still valid? (I just don't like this name 😄 )

Once @jsuarezruiz removes the help wanted tag and assigns it to you, we can remove the do not merge tag

@brminnick
Copy link
Collaborator

@VladislavAntonyuk Since this is a New Feature, could you also add Unit Tests for it?

@brminnick
Copy link
Collaborator

FYI - we are also going to wait to merge this PR until dotnet test works with .NET MAUI projects: dotnet/maui#3017

Right now, .NET MAUI projects throw this error when running dotnet test:

Testhost process exited with error: It was not possible to find any compatible framework version
The framework 'Microsoft.Maui.Core', version 'FromWorkload' (x64) was not found.

Once MAUI includes this fix, I'll update our Azure Pipelines config to include unit testing then review/merge this PR 🙌

@brminnick brminnick removed do not merge Do not merge this PR blocked labels Nov 9, 2021
@brminnick
Copy link
Collaborator

Unblocking this now that .NET MAUI supports Unit Tests 🎉

@brminnick
Copy link
Collaborator

FYI - 2 of the new UniformGrid UnitTests are failing in the CI Pipeline:

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:01.29]     CommunityToolkit.Maui.UnitTests.Views.UniformGridTests.MeasureUniformGrid [FAIL]
[xUnit.net 00:00:01.30]     CommunityToolkit.Maui.UnitTests.Views.UniformGridTests.ArrangeChildrenUniformGrid [FAIL]
  Failed CommunityToolkit.Maui.UnitTests.Views.UniformGridTests.MeasureUniformGrid [50 ms]
  Error Message:
   Assert.Equal() Failure
Expected: {Width=75 Height=75}
Actual:   {Width=-0 Height=0}
  Stack Trace:
     at CommunityToolkit.Maui.UnitTests.Views.UniformGridTests.MeasureUniformGrid() in /Users/runner/work/1/s/src/CommunityToolkit.Maui.UnitTests/Views/UniformGrid_Tests.cs:line 36
  Failed CommunityToolkit.Maui.UnitTests.Views.UniformGridTests.ArrangeChildrenUniformGrid [2 ms]
  Error Message:
   Assert.Equal() Failure
Expected: {Width=25 Height=25}
Actual:   {Width=-3.4924596548080444E-08 Height=0}
  Stack Trace:
     at CommunityToolkit.Maui.UnitTests.Views.UniformGridTests.ArrangeChildrenUniformGrid() in /Users/runner/work/1/s/src/CommunityToolkit.Maui.UnitTests/Views/UniformGrid_Tests.cs:line 44

Failed!  - Failed:     2, Passed:   275, Skipped:     0, Total:   277, Duration: 16 s - /Users/runner/work/1/s/src/CommunityToolkit.Maui.UnitTests/bin/Release/net6.0/CommunityToolkit.Maui.UnitTests.dll (net6.0)

@VladislavAntonyuk
Copy link
Collaborator Author

@brminnick , @jsuarezruiz tests are fixed.
As for the discussion about MaxRows and MaxColumns, I suggest to not put it in this PR. This one is about migration without adding a new functionality

@brminnick
Copy link
Collaborator

brminnick commented Nov 13, 2021

@brminnick , @jsuarezruiz tests are fixed.

Thanks!!

As for the discussion about MaxRows and MaxColumns, I suggest to not put it in this PR. This one is about migration without adding a new functionality

I defer to the Proposal Champion, @jsuarezruiz, but I vote to include MaxRows and MaxColumns in this PR (assuming that was the verdict in the discussion).

We really don't have the concept of "Migration PRs"; instead, we are viewing each new .NET MAUI Community Toolkit feature as a brand new feature, and if we can tweak/update/fix its similar implementation from Xamarin.CommunityToolkit, we should do it 💯

@VladislavAntonyuk
Copy link
Collaborator Author

Added as a separate commit.

@VladislavAntonyuk
Copy link
Collaborator Author

@brminnick , @jsuarezruiz could you please review? it also adds Views sample. so we do not need to copy the code between different PRs. thank you

@brminnick
Copy link
Collaborator

@jsuarezruiz - friendly reminder to review this PR ❤️

Any chance you can take a look this weekend?

@VladislavAntonyuk VladislavAntonyuk changed the base branch from main to 122-uniform-grid December 10, 2021 20:56
@VladislavAntonyuk VladislavAntonyuk merged commit 7e74233 into CommunityToolkit:122-uniform-grid Dec 10, 2021
@VladislavAntonyuk VladislavAntonyuk deleted the 122-uniform-grid branch December 10, 2021 20:57
VladislavAntonyuk added a commit that referenced this pull request Feb 14, 2022
* 122 UniformGrid

* Fix for rc2. remove compatibility usage

* Add comments, add tests

* Fix uniform tests

* MaxRows, MaxColumns

* Add samples

Co-authored-by: Brandon Minnick <[email protected]>
Co-authored-by: Pedro Jesus <[email protected]>
VladislavAntonyuk added a commit that referenced this pull request Feb 14, 2022
* 122 UniformGrid

* Fix for rc2. remove compatibility usage

* Add comments, add tests

* Fix uniform tests

* MaxRows, MaxColumns

* Add samples

Co-authored-by: Brandon Minnick <[email protected]>
Co-authored-by: Pedro Jesus <[email protected]>

Update Syntax

Update Formatting

Update Descriptions

UniformGridTestPage

Update UniformGridTestPage.xaml.cs
VladislavAntonyuk added a commit that referenced this pull request Feb 14, 2022
* 122 UniformGrid

* Fix for rc2. remove compatibility usage

* Add comments, add tests

* Fix uniform tests

* MaxRows, MaxColumns

* Add samples

Co-authored-by: Brandon Minnick <[email protected]>
Co-authored-by: Pedro Jesus <[email protected]>

Update Syntax

Update Formatting

Update Descriptions

UniformGridTestPage

Update UniformGridTestPage.xaml.cs
VladislavAntonyuk added a commit that referenced this pull request Feb 14, 2022
* 122 UniformGrid

* Fix for rc2. remove compatibility usage

* Add comments, add tests

* Fix uniform tests

* MaxRows, MaxColumns

* Add samples

Co-authored-by: Brandon Minnick <[email protected]>
Co-authored-by: Pedro Jesus <[email protected]>

Update Syntax

Update Formatting

Update Descriptions

UniformGridTestPage

Update UniformGridTestPage.xaml.cs
brminnick added a commit that referenced this pull request Feb 15, 2022
* 122 UniformGrid (#139)

* 122 UniformGrid

* Fix for rc2. remove compatibility usage

* Add comments, add tests

* Fix uniform tests

* MaxRows, MaxColumns

* Add samples

Co-authored-by: Brandon Minnick <[email protected]>
Co-authored-by: Pedro Jesus <[email protected]>

Update Syntax

Update Formatting

Update Descriptions

UniformGridTestPage

Update UniformGridTestPage.xaml.cs

* Make it more MAUI style

* Remove NSubstitute

* Rename folder from `Views` to `Layouts`

* Create `UniformItemsLayoutViewModel.cs` and `UniformItemsLayoutTestViewModel`

* Update Formatting

* Remove whitespace

* Update UniformItemsLayoutTestPage.xaml

* Update `UniformItemsLayoutTestPage`

* Remove test page

Co-authored-by: Brandon Minnick <[email protected]>
marsscotia pushed a commit to marsscotia/Maui that referenced this pull request Feb 21, 2022
* 122 UniformGrid (CommunityToolkit#139)

* 122 UniformGrid

* Fix for rc2. remove compatibility usage

* Add comments, add tests

* Fix uniform tests

* MaxRows, MaxColumns

* Add samples

Co-authored-by: Brandon Minnick <[email protected]>
Co-authored-by: Pedro Jesus <[email protected]>

Update Syntax

Update Formatting

Update Descriptions

UniformGridTestPage

Update UniformGridTestPage.xaml.cs

* Make it more MAUI style

* Remove NSubstitute

* Rename folder from `Views` to `Layouts`

* Create `UniformItemsLayoutViewModel.cs` and `UniformItemsLayoutTestViewModel`

* Update Formatting

* Remove whitespace

* Update UniformItemsLayoutTestPage.xaml

* Update `UniformItemsLayoutTestPage`

* Remove test page

Co-authored-by: Brandon Minnick <[email protected]>
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.

4 participants