Add missing script opening and closing tags in Todos.createTodoView example

[Florent2]

Add missing script opening and closing tags in Todos.createTodoView example

[jasonpgignac]

Florent2: Are you intending to submit all 15 of these commits, or just your last one about the open and close tags? I'm guessing the rest got pulled in by accident?

[Florent2]

Indeed, sorry Jason. I used the new "Edit and Fork" button, and looks like it submitted all those commits instead of my single commit. Let me know if you prefer a new pull request with my single commit, I'd redo it.

[wagenet]

No need to resubmit, just clarifying. Anyway, I'm a bit unsure about this commit. @tomdale, can you double check?

[jasonpgignac]

I concur with wagenet - I think the update is in error. The line "{{#view Todos.CreateTodoView}}" is not a piece of javascript at all, it's handlebars markdown. Am I missing something in your commit, Florent2?

[chickenkiller]

I think the modification from Florent2 makes sense if we consider the same kind of snippet which is present a bit before this one, and where the handlebars template is actually wrapped in a <script type="text/html" /> tag

@jasonpgignac, did you see the type is "text/html" and is not javascript-related at all? <script> tags don't always hold javascript.

[jasonpgignac]

@chickenkiller - However, the text in question is already in the midst of a block of html - I guess I'm failing to see what the purpose of putting the text/html box inside a body of html is? I will go through and build the app today when I get a few mins, though, with and without the script tag, and see if it makes a diff, though.

[chickenkiller]

@jasonpgignac you're right, but it's in a block of html in the previous snippet as well. So either the previous snippet should not be presented in a script tag or the current one should be, but it should be homogeneous.

I don't see the exact purpose either, maybe the aim is to parse script tags only for handlebars template and leave standard html markup unparsed (performance/integrity reason), or it may also be a semantic isolation only which has no functional effect. @tomdale should be able to answer here... And it should be interesting to add in the getting started some explanation about why this script tag is needed.

[Florent2]

Indeed, without this script tag the raw text "{{view Todos.CreateTodoView id="new-todo" placeholder="What needs to be done?"}}" is displayed in my browser (Chrome 11.0.696.71 on MacOSX), instead of the expected input text field. That's why I submitted this commit.

[jasonpgignac]

Alright, sorry for the delay, its been a long day. Since the time of your commit, the guide was already changed, to use the block view helper, it looks to me like, and this seems to work as written.

As written NOW, then, the guide seems to be correct. Does that work alright for you?

[Florent2]

Jason, I was following this guide http://guides.sproutcore20.com/getting_started.html and the current version I can see online now still misses the script tag in the code block following "update the view helper to say Todos.CreateTodoView instead of SC.TextField." at the bottom of the section "8 Creating New Todos with a Text Field".

I had made the commit to change

<h1>Todos</h1>
{{view Todos.CreateTodoView id="new-todo"
  placeholder="What needs to be done?"}}

into

<h1>Todos</h1>
<script type="text/html">
{{view Todos.CreateTodoView id="new-todo"
  placeholder="What needs to be done?"}}
</script>

I hope it makes sense, thanks for your follow-up :)

[jasonpgignac]

Ah! Now I see where the confusion is, I think. The current deployed version of the master branch is at: http://guides.sproutcore.com/getting_started.html

I believe that you're looking at the 2.0 branch at: http://guides.sproutcore20.com/getting_started.html

I think (perhaps Mr. Wagenet can confirm this?) that the 2.0 branch of the docs is supposed to refer to the 2.0 branch of sproutcore (ie, the one in the sproutcore20 repository, not 1.x as is, for instance, installed as the Sproutcore gem). Are you using the normal branch of Sproutcore or the 2.0 branch?

[Florent2]

I'm using the 2.0 branch (sproutcore-2.0.a.2).

[jasonpgignac]

So then, 1) I havent tested this with Sproutcore 2.0, and since its still very rough, we probably need to make sure the guide is documenting the expected behavior, rather than the current behavior. In that sense, @tomdale, perhaps, we could get you to confirm whether there OUGHT to be a pair of script tags - ie, is there a bug in the documentation, or a bug or mistake in the SC framework or in Florent2's attempts to get the app running on his/her machine? 2) Presumably, Florent2, you want this to be pulled into the 2.0 branch, not the master branch?

[jasonpgignac]

Also, to second what @chickenkiller said, I'd love to understand WHY the script tags are needed, if they are.

[Florent2]

Jason, indeed this commit is for the Sproutcore 2.0 guide. So yes it should be pulled into the 2.0 branch. I had wrongly assumed the master branch was for the Sproutcore 2.0 guide, I'm really sorry for the confusion.. thanks for your patience!

Let me know if you want me to redo the pull request, this time for the 2.0 branch.

[jasonpgignac]

@Florent2 - Spoke to @tomdale, and he says they are going to process your patch, AND are going to make some changes to Sproutcore20 to deal with how confusing the issue is with the script tags. So, your patch should go through in the near future AND they'll make the codebase itself less confusing, for the future. Hurrah! Thanks, and thanks @chickenkiller and @wagenet, too.

[wagenet]

I've cherry-picked 4b2b2bb into the v2.0 branch. Thanks and sorry for all the hassle!