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

Modifying iframe_sample_app to send and receive messages #64

Merged
merged 3 commits into from
Oct 21, 2014

Conversation

itirkarp
Copy link
Contributor

Earlier the iframe_sample_app used to display content from an external website (Wikipedia).

/cc @zendesk/quokka

References

Risks

  • None

@tmcinerney
Copy link
Contributor

Did you want us to review this or still WIP?

if (data.firstLoad && SIDE_BAR_REGEX.test(this.currentLocation())) {
this.showIframe({ width: '300px', height: '260px' });
handleHello: function(data) {
if (data.awesome) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This might need a comment. I think we had a problem at one point where people thought data.awesome was something we provided

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will be adding in the readme that you need to have the zaf sdk running locally. That is going to be open source so people can see in the code what is being sent in data. Do you think this data.awesome bit will be clear then?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to check for data.awesome presence if it's our own data structure? Perhaps it would be better to send something back in the data structure that we can render.

@danielbreves danielbreves changed the title WIP: Modifying iframe_sample_app to send and receive messages WIP: Modifying iframe_sample_app to send and receive messages #59 Oct 15, 2014
@danielbreves danielbreves changed the title WIP: Modifying iframe_sample_app to send and receive messages #59 WIP: Modifying iframe_sample_app to send and receive messages Oct 15, 2014

this.$form = this.$('form').eq(0);
this.formData = this.$form.serializeArray();
var message = this.formData[0].value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the input field directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did that for consistency. I looked at how other demo apps are doing similar things with forms

Copy link
Contributor

Choose a reason for hiding this comment

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

Mmm, do you remember which one by any chance? I'd would personally only do this if I was passing the whole form to be processed by some other code. We should have as little code as possible in these apps, though I know some demo apps are fairly complicated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was here: https://github.com/zendesk/demo_apps/blob/master/external_api_sample_app/app.js#L208
But I think it is passing the complete form data, like you said. So I'll simplify it here

@danielbreves
Copy link
Contributor

Perhaps it would be nice to have some representation in the UI for the iframe as well. Thoughts @zendesk/quokka?

@danielbreves
Copy link
Contributor

@maximeprades @acottrell this sample app depends on a external page that includes ZAF SDK and is able to send and receive any messages it receives. I asked @itirkarp to use the page included in ZAF SDK itself, since we're planning to open source it (soonish I'm hoping). The other alternative would be to host this somewhere, but then App Developers wouldn't be able to change its code, obviously.

@danielbreves
Copy link
Contributor

@itirkarp could you make a PR with the changes to ZAF SDK as well?

@maximeprades
Copy link
Contributor

@danielbreves as long as we're not making a lot of noise around it and if you make sure security team is aware (follow these steps: https://zendesk.atlassian.net/wiki/display/ENG/Open+Source) i'm ok with open sourcing that repo if you think it's ready.

The only thing is I want to "relaunch" it properly in Q1, with some marketing calories behind it as a brand new feature so that's why I don't want to make noise about it now (i.e blog post etc...)

@danielbreves
Copy link
Contributor

@maximeprades awesome! I've added a couple of killer LAB stories that we could use to add even more "calories" for the launch in Q1: LAB-14, LAB-15

@maximeprades
Copy link
Contributor

@danielbreves are the URLs final? I remember some discussions around serving them from the CDN for better perf which would result in a URL change and thus breaking apps for people.

@danielbreves
Copy link
Contributor

@maximeprades
Copy link
Contributor

ok what about URLs? are they final? what are they? sorry I suppose i could RTFM 😄

@danielbreves
Copy link
Contributor

@maximeprades totally haha -> https://developer.zendesk.com/apps/docs/agent/iframes_in_apps
final is a strong word tough, I don't know, I don't have all the answers!! 🙀


This app aims to be a good start to learn how to iframe things of different sizes and methods.
This app aims to be a good start to learn how send messages between an iframe and an app.
Copy link
Contributor

Choose a reason for hiding this comment

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

, using the [Zendesk App Framework SDK]()?

@itirkarp
Copy link
Contributor Author

@danielbreves
PR for ZAF SDK changes: zendesk/zendesk_app_framework_sdk#21

if (data.firstLoad) {
this.showIframe({ width: '1024px', height: '500px' });
handleMessageReceived: function(data) {
if (data.awesome) {
Copy link
Contributor

Choose a reason for hiding this comment

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

data.awesome was just to demonstrate the ability to pass data around, since we're already using data.message below we can probably get it of data.awesome everywhere else.

@itirkarp
Copy link
Contributor Author

💇
/cc @zendesk/quokka
Ready for review

@itirkarp itirkarp changed the title WIP: Modifying iframe_sample_app to send and receive messages Modifying iframe_sample_app to send and receive messages Oct 21, 2014
@itirkarp itirkarp removed the wip label Oct 21, 2014
@danielbreves
Copy link
Contributor

👍


</div>

{{iframe "http://localhost:9001"}}
Copy link
Contributor

Choose a reason for hiding this comment

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

\n OCD

@sandlerr
Copy link
Contributor

any chance of a screenshot / licecap gif?

@itirkarp
Copy link
Contributor Author

@sandlerr There is a screenshot in the readme

@sandlerr
Copy link
Contributor

👍


This app aims to be a good start to learn how to iframe things of different sizes and methods.
This app aims to be a good start to learn how send messages between an iframe and an app using the [Zendesk App Framework SDK](https://github.com/zendesk/zendesk_app_framework_sdk)

Choose a reason for hiding this comment

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

...learn how to send...

?

@itirkarp itirkarp force-pushed the itirkarp/demo-chat-with-zafsdk branch from 8464ffa to d53b56f Compare October 21, 2014 23:11
itirkarp added a commit that referenced this pull request Oct 21, 2014
Modifying iframe_sample_app to send and receive messages
@itirkarp itirkarp merged commit b248960 into master Oct 21, 2014
@itirkarp itirkarp deleted the itirkarp/demo-chat-with-zafsdk branch October 21, 2014 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants