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.

Sign up for GitHub

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

Jump to bottom

Implementation of LazyView + Sample + Unit Tests #1102

Merged
merged 9 commits into from
Mar 29, 2023
Merged

Implementation of LazyView + Sample + Unit Tests #1102

merged 9 commits into from
Mar 29, 2023

Conversation

kphillpotts
Copy link
Contributor

@kphillpotts kphillpotts commented Mar 19, 2023 •
edited
Loading

Description of Change

This pull request migrates LazyView from XCT, along with unit tests and page in the Sample App

Linked Issues

PR Checklist

Additional information

@kphillpotts kphillpotts changed the title Implementation of LazyView + Samples = Unit Tests Implementation of LazyView + Sample + Unit Tests Mar 19, 2023
@kphillpotts kphillpotts self-assigned this Mar 19, 2023
kphillpotts and others added 4 commits March 19, 2023 17:37
@kphillpotts @VladislavAntonyuk
Update samples/CommunityToolkit.Maui.Sample/ViewModels/Views/ViewsGal…
9a69ef3 
…leryViewModel.cs


Nice catch - 🤦

Co-authored-by: Vladislav Antonyuk <33021114+VladislavAntonyuk@users.noreply.github.com>
@kphillpotts @VladislavAntonyuk
Update src/CommunityToolkit.Maui/Views/LazyView/BaseLazyView.shared.cs
35c7f47 
Co-authored-by: Vladislav Antonyuk <33021114+VladislavAntonyuk@users.noreply.github.com>
@kphillpotts
Update unit tests to correctly await async methods
71c00a2
@kphillpotts
Fixing issue with mismatched brackets.
dd4fc42
@brminnick brminnick added pending documentation This feature requires documentation do not merge Do not merge this PR labels Mar 20, 2023
@brminnick
Copy link
Collaborator

Thanks Kym!! I've added the pending documentation This feature requires documentation and do not merge Do not merge this PR labels to ensure we don't forget to write the docs.

Just FYI - I made a couple tweaks:

  • Changed public abstract class BaseLazyView -> public abstract class LazyView
    • Since it's an abstract class, the user will get a compiler error if they try to instantiate it or use in XAML
  • Changed LazyView.IsLoaded -> LazyView.HasLazyViewLoaded
    • It was an ok variable name for XCT, but we can't use IsLoaded in CommunityToolkit.Maui because MAUI added a new property to VisualElement, VisualElement.IsLoaded, and we don't want to block that property

I also fixed the namespace for LazyViewTests.cs and whilst doing so I noticed a few other Unit Tests were using incorrect namespaces, so I fixed those too.

@bijington
Copy link
Contributor

@kphillpotts Great work on this! Did you want any help with the docs writing?

@kphillpotts
Copy link
Contributor Author

@kphillpotts Great work on this! Did you want any help with the docs writing?

I'll give them a shot myself to check I have correct permissions and processes - mostly they will be moved from XCT anyway. But I'll reach out if I have questions. Thanks!

@jfversluis jfversluis removed pending documentation This feature requires documentation do not merge Do not merge this PR labels Mar 29, 2023
@jfversluis
Copy link
Member

Added the Docs PR so I think we're good to go with this one?

pictos
pictos approved these changes Mar 29, 2023
View reviewed changes
Copy link
Member

@pictos pictos left a comment

Choose a reason for hiding this comment

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

jfversluis
jfversluis approved these changes Mar 29, 2023
View reviewed changes
Copy link
Member

@jfversluis jfversluis left a comment

Choose a reason for hiding this comment

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

Just the one minor thing, otherwise merge away!

samples/CommunityToolkit.Maui.Sample/Pages/Views/LazyView/CustomLazyView.cs Show resolved Hide resolved
@jfversluis jfversluis merged commit 0df52ee into CommunityToolkit:main Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brminnick brminnick brminnick left review comments

@jfversluis jfversluis jfversluis approved these changes

@VladislavAntonyuk VladislavAntonyuk VladislavAntonyuk approved these changes

@pictos pictos pictos approved these changes

Assignees

@kphillpotts kphillpotts

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Proposal] LazyView
6 participants
@kphillpotts @brminnick @bijington @jfversluis @pictos @VladislavAntonyuk