Sorry... it was in the back of my head, but now...

I see no fundamental problem with it. Some minor notices:

I think it's unnecessary to move the immediate execution functionality
out to js/execute.js, especially as there's many other "ready"
functionality just bellow it, inside that "script" element directly.
It could fit on there.

Also I guess it would be more reliable if the immediate execution
action is the last among the "ready" actions, because that's when it
happens in the normal (non-Manual) case too.

Some JavaDoc about FreeMarkerOnlineView constuctor paramteters,
especially about `execute` wouldn't hurt at this point.

I see your pull request is still pending... Well, after 14 days or so
I will start asking about it. Also when this current thing is ready,
push it too.

And yes, you in general you are supposed to work in feature branches
and push those, otherwise you append to the last "master" push.
However, I say, this current work can be appended to it, as they
aren't really separate features.

-- 
Thanks,
 Daniel Dekany


Monday, September 21, 2015, 7:43:08 PM, Pradeep Murugesan wrote:

> Daniel,
>  got a chance to review the changes ?
> Pradeep.
>
>> From: [email protected]
>> To: [email protected]
>> Subject: RE: FM-Online connect with Manual
>> Date: Sat, 19 Sep 2015 22:52:11 +0530
>> 
>> Hi Daniel,
>>   the POC worked. So I have made the changes to suit our idea of triggering 
>> Ajax when we click from manual.
>> I have removed the non ajax code and also modified the unit test cases. 
>> Removed a file that is not used after the changes.
>> https://github.com/pradeepmurugesan/freemarker-online/commit/d90745d91d94503f444fab8ac41d8a0443ac5794
>> https://github.com/pradeepmurugesan/freemarker-online/commit/546a73cb4476a8b981858825c907704ece36973d
>> I have checked these into another branch(FreeMarker_Manual_Try_it_out) since 
>> the first pull request is not accepted by them yet and I am not sure If I 
>> push these changes it will go as part of existing pull request.
>> Kindly review and let me know if any changes.
>> Pradeep.
>> 
>> > Date: Wed, 16 Sep 2015 08:39:36 +0200
>> > From: [email protected]
>> > To: [email protected]
>> > Subject: Re: FM-Online connect with Manual
>> > 
>> > I agree with what you have described.
>> > 
>> > You have earlier said that this will work by first rendering the page
>> > with the filled form, and then sending the AJAX request. That's the
>> > most maintainable way of doing this, as it requires almost no
>> > duplicated functionality. So that's maybe fine. But the more efficient
>> > (lowest response time) way off doing this is also rendering the
>> > response together with the filled form, so there's no AJAX request.
>> > 
>> > 
>> > Wednesday, September 16, 2015, 7:05:58 AM, Pradeep Murugesan wrote:
>> > 
>> > > Yeah I think the Cross Domain issues comes into play with
>> > > javascript. This approach should work. Let me do a POC and come back if 
>> > > its not working.
>> > > Meanwhile please let me know if we have different visualizations.
>> > > Pradeep.
>> > >
>> > >> From: [email protected]
>> > >> To: [email protected]
>> > >> Subject: RE: FM-Online connect with Manual
>> > >> Date: Wed, 16 Sep 2015 10:22:32 +0530
>> > >> 
>> > >> > <form target="manualFMOnline" action="..." method="post">...</form>
>> > >> Hmmm but the above code will be in our manual , we should get 
>> > >> redirected to the FM-Online domain right. That's where I am doubting 
>> > >> whether cross domain POST will be allowed through browser redirection.
>> > >> Ok Even before going there , Let me tell my understanding so that lets 
>> > >> check if we are on the same page.
>> > >> 1. FM Manual website will have a button or a link saying "try it out" 
>> > >> or something like that.2. When the user clicks on the same he will 
>> > >> redirected to a new page(FM-Online) where the corresponding template 
>> > >> and datamodel will be prefilled and executed.
>> > >> Am I right regarding this ?
>> > >> Kindly let me know if  you have visualised something different ?
>> > >> Pradeep.
>> > >> 
>> > >> > Date: Tue, 15 Sep 2015 22:39:21 +0200
>> > >> > From: [email protected]
>> > >> > To: [email protected]
>> > >> > Subject: Re: FM-Online connect with Manual
>> > >> > 
>> > >> > Huh? I meant:
>> > >> > <form target="manualFMOnline" action="..." method="post">...</form>
>> > >> > 
>> > >> > 
>> > >> > Tuesday, September 15, 2015, 11:39:49 AM, Dékány Dániel wrote:
>> > >> > 
>> > >> > > Won&#39;t something like <format target="manualFMOnline"
>> > >> > > method="post">...</form> work? Anyway, I think both POST and GET
>> > >> > > should do the same, but using POST should be the norm. With GET you
>> > >> > > can get some very long URLs. While URL-s up to 2K length used to
>> > >> > > work on most places, even URL-s over 256 bytes is sometimes 
>> > >> > > considered worrisome.
>> > >> > > Pradeep Murugesan <[email protected]> írta:
>> > >> > >>Hi Daniel,
>> > >> > >> The initial idea was when people click from manual we POST to the 
>> > >> > >> "/" with template and dataModel and get the page rendered 
>> > >> > >> directly. We do not have any direct method to open a new tab and 
>> > >> > >> directly post data. Some round about ways were mentioned in web 
>> > >> > >> but nothing concrete.
>> > >> > >>So I thought of
>> > >> > >>1. Issuing a GET request to fmonline/ with formdata. This will help 
>> > >> > >>us to populate the fields.2. We will also see if formdata is not 
>> > >> > >>empty then we will call our execute via ajax. 3. If formdata is 
>> > >> > >>empty we will know that its a normal GET request (i.e not from 
>> > >> > >>manual) and render the empty page.
>> > >> > >>
>> > >> > >>Let me know your thoughts.
>> > >> > >>Pradeep.
>> > >> > >>
>> > >> > >>
>> > >> > >>
>> > >> > >>
>> > >> > >>          
>> > >> > 
>> > >> > -- 
>> > >> > Thanks,
>> > >> >  Daniel Dekany
>> > >> > 
>> > >>                                         
>> > >                                           
>> > 
>> > -- 
>> > Thanks,
>> >  Daniel Dekany
>> > 
>>                                         
>                                           

Reply via email to