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

Added Windows 10 Universal App Support #72

Merged
merged 2 commits into from Mar 15, 2016

Conversation

Projects
None yet
5 participants
@Vo2le
Contributor

Vo2le commented Mar 1, 2016

Its not much code and i hope i haven't make any mistakes on upload.
Maybe i can have some feedback if someone successfully tested the code.

@agarrharr

This comment has been minimized.

Member

agarrharr commented Mar 1, 2016

This should definitely be tested if anyone has a Windows 10 phone.

Also, one piece of feedback though, Almost every line in plugin.xml is changed (due to indentation), so it's really hard to tell what has changed in this file. Also, it didn't need to be indented because the tag at the top isn't a normal tag and it doesn't have a closing tag. Could you update your pull request?

To do this:

  1. Change the file
  2. Run git commit --amend (this will open up your text editor)
  3. Save and close the file that was opened
  4. Run git push -f

@Vo2le Vo2le force-pushed the Vo2le:master branch from 16b0c68 to 5136ab0 Mar 1, 2016

@Vo2le

This comment has been minimized.

Owner

Vo2le commented on 5136ab0 Mar 1, 2016

Yeah, my beautifier has changed the linebreaks so git changed whole file.

I tested the code already on Windows Phone 10.
On Windows 10 Desktop it also works and tries to open a messenger app.
I just hope i doesnt forget to add something, but in my opinion it should work.

This comment has been minimized.

agarrharr replied Mar 2, 2016

Great! That looks a lot better. 👍

@agarrharr

This comment has been minimized.

Member

agarrharr commented Mar 2, 2016

I'll leave it up to @dbaq, if he wants to merge it. I don't know if we'll be able to find anyone else to test it on a windows phone.

@dbaq

This comment has been minimized.

Member

dbaq commented Mar 2, 2016

Thanks @Vo2le for contributing and @aharris88 for reviewing. Maybe @brodybits can try before we merge this? I have no way to test that code. @brodybits, are you please available to test this code?

it is related to #31.

@brodybits

This comment has been minimized.

Contributor

brodybits commented Mar 2, 2016

@brodybits, are you please available to test this code?

it is related to #31.

@dbaq unfortunately I do not have a Windows 10 phone and am still Visual
Studio 2013 (Express). I am also really busy with commitments this week and
will probably upgrade to Visual Studio 2015 in the next 1-2 weeks.

I can try this on a Windows Phone 8.1 simulator tonight and report if it
works there or not. If it works on Windows Phone 8.1 I will recommend that
you accept this as an "alpha" or "pre-alpha" version (with a note in the
README) and let the community help with further testing.

I can also look for others who could help with Windows 10 testing.

@dbaq

This comment has been minimized.

Member

dbaq commented Mar 2, 2016

That would be great, thank you @brodybits. Keep us posted.

@brodybits

This comment has been minimized.

Contributor

brodybits commented Mar 2, 2016

@dbaq

This comment has been minimized.

Member

dbaq commented Mar 2, 2016

Thanks for the quick feedback. I'll be waiting for your feedback before merging it. Thanks.

@sachingarg2k1

This comment has been minimized.

sachingarg2k1 commented Mar 11, 2016

I tested this code on Windows 8.1 phone. VStudio refuses to register the package once I add this plugin. Error:
Error : DEP0001 : Unexpected Error: Package could not be registered. (Exception from HRESULT: 0x80073CF6) CordovaApp.Phone

@brodybits

This comment has been minimized.

Contributor

brodybits commented Mar 11, 2016

I wonder if you need to specify the capability in the manifest?

@Vo2le

This comment has been minimized.

Contributor

Vo2le commented Mar 11, 2016

It seems you are right. After it worked i haven't thought about that. I testet it without the chat capability and it seems to work anyway. I will delete that lines.

I don't think this code will run on Windows 8 devices.

Removed windows 10 chat capabilities
is not needet to write sms
@brodybits

This comment has been minimized.

Contributor

brodybits commented Mar 14, 2016

I just tested the version by @Vo2le on a new Windows 10 mobile device and it does start the SMS composition interface. (I have not tested the callbacks since I am not using it with a SIM right now.)

I recommend that you include this change and announce it in README.md so others can try it out.

@dbaq

This comment has been minimized.

Member

dbaq commented Mar 15, 2016

Thanks @brodybits and everybody else for testing this PR. Merging it. 👍 🍻

dbaq added a commit that referenced this pull request Mar 15, 2016

Merge pull request #72 from Vo2le/master
Added Windows 10 Universal App Support

@dbaq dbaq merged commit 2f6f78b into cordova-sms:master Mar 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment