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

Add new apps and fixed a typo in MessagesTest.swift #28

Merged
merged 2 commits into from
Jan 4, 2016
Merged

Add new apps and fixed a typo in MessagesTest.swift #28

merged 2 commits into from
Jan 4, 2016

Conversation

Maryom
Copy link
Contributor

@Maryom Maryom commented Jan 4, 2016

Salam :)

I added Sonos, Videos, OneDrive, and Outlook. I would like to add more apps for Microsoft I think it will be a great idea do you agree with me?

But the problem is app path I think it not suite with Appz, Do you agree?
This is what I found from documentation:
https://msdn.microsoft.com/en-us/library/office/dn911482.aspx
https://msdn.microsoft.com/it-it/library/office/dn906146.aspx#sectionSection9

I tried to add OneNote, Word, Excel, and PowerPoint all of them opened but they keep showing a warning message that I have to open them with a specific file.

It's easy to do it as I found in the above links, like this:
Schema format:
|u||p||c|
The following example shows a request to invoke a Word file for editing:
ms-word:ofe|u|https://contoso/Q4/budget.docx|p|clouddrive|c|folderviewQ4

But what should we do with ofe|u| we have instead of it //app, Any suggestions?
Thanks ..

@Mazyod
Copy link
Member

Mazyod commented Jan 4, 2016

Hi, thanks for adding these, I think they will be a valuable addition to Appz!

I fully understand your concern about the format of the URL, and it can easily be fixed. Appz is a very flexible and modular library, and you'll see how in a bit. This is not a trivial task, and will require some thought and design process, so be prepared for that :)

Instead of trying to make the parameters passed into the action directly, we can add a new struct to build the URL for us:

public struct MSPayload {
    public var url: String
    public var passback: String? = nil
    public var context: String? = nil

    public func buildURL() -> String {
        var sections = [("u", url)]
        if let passback = passback {
            sections.append(("p", passback))
        }
        if let context = context {
            sections.append(("c", context))
        }
        // convert tuples to "|x|string", then join them
        return sections.map { "|\($0.0)|\($0.1)" }.joinWithSeparator("")
    }
}

Then, you can make the user pass in a MSPayload to the MS apps:

enum Action {
    case Open(MSPayload)
}
...
switch action {
case .Open(let payload):
    return payload.buildURL()
}

I apologize for any mistakes, I wrote this directly on github, but I think it's enough to get the idea. I hope this helps, and you can try to write tests for the buildURL function as well.

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod Great suggestion, thanks :) I will try to add it.

@Mazyod
Copy link
Member

Mazyod commented Jan 4, 2016

I don't have an account, unfortunately. Azerbaijan is a nice country, you can leave it as it is and create an account :) ?

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod When I sent you my question, the web suddenly worked with Kuwait that why I deleted it. Thanks for your fast reply 🚀

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod Can I tell you the problem?

@Mazyod
Copy link
Member

Mazyod commented Jan 4, 2016

@Maryom Sure, what happened?

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod Thank you so much. I was trying to get the direct URL for a document for 2 hours 🙃 all my tries failed. I understand this:
ms-word:ofe|u|https://contoso/Q4/budget.docx|p|clouddrive|c|folderviewQ4

From the document I understand that these are optional: |p|clouddrive|c|folderviewQ4

So no need for them, I removed them and this what I get in Xcode:
url Optional(ms-word://ofe%7Cu%7Chttps%3A%2F%2Fd.docs.live.net%2Ffa74248df401c361%2Ffun.docx)

It looks great but I can't figure why it still not working?

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod From this blog, I thought I have to use Office 365
http://blog.beecomedigital.com/2015/02/16/opening-a-document-with-office-mobile-from-your-own-application/

But when I used it nothing is change!

In the above post, from where they got this URL: https://tenant.sharepoint.com/Shared%20Documents/Document.docx

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod I tried right click share, and I tried to email it to myself and I tried embedded link all links didn't work.

@Mazyod
Copy link
Member

Mazyod commented Jan 4, 2016

@Maryom Thanks, I'll try to look into this soon

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod I think no need to look at it, because I noticed that in MS apps if you try to open for example a word file in one drive app it will open perfectly but not directly it shows downloading and then open it. Do you agree with me? I wish I can record it for you but it's on my iPhone.

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod Like this pic.
img_2167

@Mazyod
Copy link
Member

Mazyod commented Jan 4, 2016

@Maryom You can always push basic working implementation, then we can iterate on it to make it better 👷

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod Also, I was thinking that no need for // because in the document there is no // after ms-word:

ms-word:ofe|u|https://contoso/Q4/budget.docx|p|clouddrive|c|folderviewQ4

So I removed them but it not working at all
This is the URL without //
ms-word:ofe%7Cu%7Chttps%3A%2F%2Fd.docs.live.net%2Ffa74248df401c361%2Ffun.docx

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod Do you suggest to PR with it even it showing warning message? it's really bad PR, because I couldn't solve the problem 😔

@Mazyod
Copy link
Member

Mazyod commented Jan 4, 2016

@Maryom Yeah, the double slash is always required. It's fine, I will not make a release until we make this working. merging a PR doesn't mean we will make a new release :)

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod Ok I will try more until 12:00 am if it not working I will PR. I have a note on Mail.swift in public struct Email I think init should be public:

public init(recipient: String = "", subject: String = "", body: String = "") {

    self.recipient = recipient
    self.subject = subject
    self.body = body
}

This is the error when it not public:
screen shot 2016-01-04 at 10 10 39 pm

But when I made public it worked fine, Do you agree with me?

@Mazyod
Copy link
Member

Mazyod commented Jan 4, 2016

@Maryom Yeah, Email init should definitely be public

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod إن شاء الله I will fix it with the next PR. Thank you for being there while I'm lost! I hope I can fix my problem by myself so that I give something useful ..

…Also, I made init email struct a public init
@Mazyod
Copy link
Member

Mazyod commented Jan 4, 2016

@Maryom Thanks for pushing this, I'll look into why it's not working, I'm sure it's just a small problem with a configuration somewhere

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod Your welcome. This just pop up, what should I do?

screen shot 2016-01-05 at 12 24 09 am

@Mazyod
Copy link
Member

Mazyod commented Jan 4, 2016

@Maryom I don't know, maybe you should reload the browser or something?

@Maryom
Copy link
Contributor Author

Maryom commented Jan 4, 2016

@Mazyod Me too I don't know. I hope you delivered the PR. If not please tell me to resend it.

Mazyod added a commit that referenced this pull request Jan 4, 2016
Add new apps and fixed a typo in MessagesTest.swift
@Mazyod Mazyod merged commit f2a3008 into SwiftKitz:master Jan 4, 2016
@Mazyod Mazyod mentioned this pull request Jan 5, 2016
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.

None yet

2 participants