unoplatform / uno

Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported.
https://platform.uno
Apache License 2.0
8.51k stars 690 forks source link

Contacts and android reader #2544

Open pkar70 opened 4 years ago

pkar70 commented 4 years ago

GitHub Issue (If applicable): #3045

PR Type

What kind of change does this PR introduce?

What is the current behavior?

Nothing in Windows.ApplicationModel.Contacts is implemented.

What is the new behavior?

Parts of this namespace is implemented. See details in "Other information" section.

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Code, for Android only:

Part of my apps:

Internal Issue (If applicable):

gitpod-io[bot] commented 4 years ago

pkar70 commented 4 years ago

Is there something I can do with CI build error: NuGetCommand

The nuget command failed with exit code(1) and error(Response status code does not indicate success: 500 (Internal Server Error - VS800075: The project with id 'vstfs:///Classification/TeamProject/1dd81cbd-cb35-41de-a570-b0df3571a196' does not exist, or you do not have permission to access it. (DevOps Activity ID: DB1612C5-FFAE-43A2-8EA1-DF4EC0737116)).)

CLAassistant commented 4 years ago

CLA assistant check
All committers have signed the CLA.

pkar70 commented 3 years ago

Seems like all build errors are outside this PR. Can you restart build?

pkar70 commented 3 years ago

Build problem: "Build\Uno.UI.Build.csproj(322,3): Error MSB3073: The command "C:\a\1\s\Build\tools\generatepkgdiff.exe --base=Uno.UI --other=Uno.UI.3.2.0-PullRequest-2544-21086-1-2544.300.nupkg --diffignore=PackageDiffIgnore.xml --outfile=C:\a\1\a\ApiDiff.3.2.0-PullRequest-2544-21086-1-2544.300.md" exited with code -532462766" Could build be restarted?

pkar70 commented 3 years ago

could you restart this build?

MartinZikmund commented 3 years ago

/azp run

azure-pipelines[bot] commented 3 years ago
Azure Pipelines successfully started running 1 pipeline(s).
pkar70 commented 3 years ago

Why "The member IRandomAccessStreamReference Contact.SmallDisplayPicture is not implemented in Uno" is from my PR? I didn't touch SmallDisplayPicture,,,

jeromelaban commented 3 years ago

Thank you for your PR. As with all PRs that you have opened, we need ways to validate that what you've created works. Can you add samples in the sample app, along with scenarios to validate what's supposed to work ?

pkar70 commented 3 years ago

Yes, I can. But when I added test, in CalendarDatePicker (#2079), it also didn't get any reaction. First, I want to have one checked/accepted/merged PR with such manual test, before I "clone" it to my other PRs.

jeromelaban commented 3 years ago

You won't need to clone the work, samples will be different as the APIs are different. You'll need working samples for all PRs that add significant work.

pkar70 commented 3 years ago

I know how to write automated test, and made many of them in my PRs. But I never done correct manual test. I tried to make such test many times, always it was wrong. Newest try is in CalendarDatePicker (#2079). If that test is ok, I can make some tests for my other PRs. But first I want to have one test checked and with some feedback that test is ok.

Although I don't know if e.g. in this PR (contacts) I can assume that some contacts are defined. Same problem for call history...

pkar70 commented 2 years ago

What can be tested? it is only reader, and default installation of Android has empty contact list.

gitpod-io[bot] commented 2 years ago

pkar70 commented 2 years ago

What can be tested? it is only reader, and default installation of Android has empty contact list.

pkar70 commented 2 years ago

Will build restart by itself, or should it be restarted manually?

jeromelaban commented 2 years ago

When running the tests, I just realized that @MartinZikmund already made an implementation of ContactPicker which contains most of what you're adding in this PR. It would be wiser to reuse or adjust the android contact reading code to be reused in ContactsReader.

pkar70 commented 2 years ago

What do you mean by "most of what you're adding in this PR"? ContactReader, ContactManager, and ContactStore are not implemented at all, this PR creates such files, not changing them. Only ContactPicker is implemented, and this is completely different thing. You get only one Contact from picker (contact selected by user), and my PR is about reading all contacts without any UI involved.

jeromelaban commented 2 years ago

The reading of invidual contacts is implemented, which looks to be duplicated code from what you are adding. Can you try reusing the code instead?

pkar70 commented 2 years ago

Reading contact is possible either using ContactReader (from my PR, getting list of all contacts known to operating system), or getting one contact selected by user using native OS dialog. If you mean "integrating" SampleApps, yes, it can be done, but: a) integrated sample would have two buttons, one to invoke Picker, second - to read list of contacts from system; also information about count of contacts is only present in ContactReader (not in ContactPicker) b) my sample can be easily converted to read/write (if anyone implements contact writer), c) Picker sample is to test UI, and Reader sample is to test no-UI.

Should I combine two samples into one, or can it be as it is?

jeromelaban commented 2 years ago

I'm not talking about combining the samples. I'm saying the code you wrote to implement ContactReader is mostly already present in ContactPicker (see here https://github.com/unoplatform/uno/blob/56d3ba98ee6cab055dc8d7c4941ce4494e20e946/src/Uno.UWP/ApplicationModel/Contacts/ContactPicker.Android.cs). Take a look there and see what's the same and let's share the code between both classes.

GitHub
uno/ContactPicker.Android.cs at 56d3ba98ee6cab055dc8d7c4941ce4494e20e946 · unoplatform/uno
Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - uno/ContactPicker.Android.cs at 56d3ba98ee6cab055dc8d7c4941ce4494e20e946 · unoplatfor...
pkar70 commented 2 years ago

Ok. By the way, this PR is year-and-a-half older than MartinZikmund PR :) Changing some parts of ContactPicker is required.

pkar70 commented 2 years ago

I'll try to check this null reference exception after Resurrection - by creating own build of Uno, based on current Uno and this PR, and using it in my app, or I would try to learn how to build Uno with samples and use it in emulator.

pkar70 commented 2 years ago

Seems like after requested changes, no email nor phone numbers are returned. Will check it tomorrow.

pkar70 commented 2 years ago

Reverting (partly) last commit (using ContactPicker to read contact data), so now from ContactPicker only conversions between Android and UWP (enum) types are used. Also, I switched from my internal class members to ContactQuerySearchText (two fields defined for all platforms, one field still unimplemented for all). I don't know why MartinZikmund uses some other contactId, what is this contactId and what is relation between his contactId and my contactId. But now reading all contact data works (in ContactReader).

pkar70 commented 2 years ago

Can build be restarted?

pkar70 commented 2 years ago

Ok, now sample shows full list of contacts (as ListView)

pkar70 commented 2 years ago

could build be restarted?

pkar70 commented 2 years ago

Can it be accepted/merged?

pkar70 commented 2 years ago

Can build be restarted, and auto-merge re-enabled?

agneszitte commented 2 years ago

/azp run

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 2 pipeline(s).
agneszitte commented 1 year ago

@pkar70 there are some conflicts that need to be resolved for this PR ;)