Updated project to TroopJS 2.x

[mikaelkaron]

Same application, next generation of TroopJS library.

PS. I've checked in all of the bower_components except troopjs-bundle, I cleaned that to only include the files I use, should I do that with the other ones to?

[stephenplusplus]

Awesome!

I don't have a chance to dive in too deep just yet, but I wanted to mention a few small things I saw:

• We only include the files that are essential to the application in bower_components/____, i.e., just bower_components/dist/build/plugin.js (path left as is), and not the readmes, tests, and other stuff that bower installs.
• There are some small code style stuff. See the code style guide. If you run jshint js/ from the troop directory, you'll see the easily-fixable violations listed.
• Double clicking on the ✔ put me in editing mode. It should just toggle the todo on and off
• When entering edit mode from a dblclick, the cursor should be placed at the end of the input
• You can edit /learn.json, instead of creating a new one. Please update the troopjs readme with any additions you make as well.

After a closer look, I'll come back with anything else I might run into. Thanks again for keeping the app fresh!

[mikaelkaron]

I've cleaned bower_components, updated the global learn.json and updated the code to pass jshint with some exceptions.

• In TroopJS we don't have to new components, so I added a jshint rule to ignore that where applicable.
• I have a warning for the keyword yield that's coming from when.js, I won't argue if this is truly a reserved word the way it's used, but that's what the library uses, so that's what I have to do.

In the app specs I see

When editing mode is activated it will hide the other controls and bring forward an input that contains the todo title, which should be focused (.focus()). The edit should be saved on both blur and enter, and the editing class should be removed. Make sure to .trim() the input and then check that it's not empty. If it's empty the todo should instead be destroyed. If escape is pressed during the edit, the edit state should be left and any changes be discarded.

Not really sure how I'd both focus and move the cursor...

[passy]

You can also remove the minified versions from the bower_components.

[passy]

Remove

[mikaelkaron]

I looked at this from the app specs

Use minified version of third-party code and put it in the lib folder.

And just applied that to bower_components. So I guess:

To minify or not to minify, that is the question.

[passy]

Uh, we really need to update those. We use unminified nowadays to make stepping through the code easier.

[stephenplusplus]

In TroopJS we don't have to new components, so I added a jshint rule to ignore that where applicable.
I have a warning for the keyword yield that's coming from when.js, I won't argue if this is truly a reserved word the way it's used, but that's what the library uses, so that's what I have to do.

You only have to make your application code pass JSHint. No need to worry about the libraries. Just test your js/ directory.

Not really sure how I'd both focus and move the cursor...

Take a look at other examples e.g. angular to see this in action. Feel free to snoop around the code of the other projects to see how they've accomplished this.

[mikaelkaron]

You only have to make your application code pass JSHint. No need to worry about the libraries. Just test your js/ directory.

I'm using .yeild in my code, the method comes from when.js

Take a look at other examples e.g. angular to see this in action. Feel free to snoop around the code of the other projects to see how they've accomplished this.

The example you provided acts different in different browsers for me.

• In firefox the cursor stays in the beginning of the word.
• In chrome the cursor is moved to the end of the word.

In both cases the .focus() is removed. My understanding of why .focus() was even there is that if you start typing right away you remove the previous text. I have to admit that this is only tested on my linux box, perhaps the result is different on other platforms?

[mikaelkaron]

No. Just "borrowed" a chrome and a firefox session from a fellow developer that uses windows and the same behaviour exists for him. Maybe I'm misunderstanding the original problem.

When entering edit mode from a dblclick, the cursor should be placed at the end of the input

When the app specs say

When editing mode is activated it will hide the other controls and bring forward an input that contains the todo title, which should be focused (.focus())

Granted this could mean that the input box should be focused and the text not selected, then again it could mean that you first set the input value and then focus (keeping the selection).

I took a quick look at some of the examples and there are various behaviours noticed. Dojo, Knockback, Montage and Serenade focuses (and selects) the text after dblclick, while most of the others would simply move the cursor to the where it was last time the input element was used (I think this is why).

Either way, I personally like that the text gets selected for the reason stated above, and since the specs don't really mention anything about cursor position, is it ok to just leave as is?

[stephenplusplus]

Thanks for taking the time to look into the focus stuff. It is a relatively new addition, and one that has been implemented differently across the demonstrations, or not implemented at all yet. Using Chrome, I picked the Angular example, as the cursor moving to the end of the line is the intended behavior.

We are still working on various tweaks and patches, so I'll be sure to add cleaning this up as well to the list.

[mikaelkaron]

By the way, in c67b1ee I took the liberty of updating index.html with more up to date information about TroopJS as well as move the project to troopjs_require as that's more in line with the other projects of the same category.

[mikaelkaron]

So except for the .yeild, are there any other things I can do to get this merged?

[stephenplusplus]

I saw a few things pop up in a few places, so generic code style rules:

define([ 'troopjs-browser/component/widget', 'jquery' ], function MarkModule(Widget, $) {
// should be...
define([
    'troopjs-browser/component/widget',
    'jquery'
], function MarkModule(Widget, $) {

methodName : function methodName() {
// should be...
methodName: function methodName() {

if () {

}
else if () {
}
else {
}
// should be...
if () {
} else if () {
} else {}

[stephenplusplus]

You can use Array.filter here var count = items.filter(filter).length;

[mikaelkaron]

Ok, then I guess I'll have to pull in a poly lib on top of that (to get all your Array stuff. ;) no worries.

[mikaelkaron]

methodName : function methodName() {
// should be...
methodName: function methodName() {

I see the extra space, but I can't find where that's mentioned in the code style or idiomatic.js unless I guess from unrelated examples.

if () {
}
else if () {
}
else {
}
// should be...
if () {
} else if () {
} else {}

Here I can find a reference in idiomatic.js but I feel a bit odd having to dig through (granted no a lot of) documentation to choose how to format my if statements. If this is truly important it would be better to just have a tool that formats the code, I believe you can run this through uglify without stripping comments and with no compression to get consistent formatting (not sure it will look good, but maybe it's worth a try)

[mikaelkaron]

Looks like there are no obvious tools available (at least not that gives you the granularity you are looking for) but building one on top of esformatter should be possible. Maybe adding a ticket for a formatting tool?

[mikaelkaron]

So that's about it, if and else, spaces before : and define statements. Anything else?

[stephenplusplus]

Thanks for your patience on all these code style requests. This project is the culmination of a multitude of frameworks and even more contributing developers, each coming with their own personal coding style. We are still chasing a code style guide that encompasses everything, but it's sometimes necessary to look to the other examples. The strictness regarding code style is for the benefit of the user coming to compare frameworks. When looking at two examples side-by-side, their attention shouldn't be drawn to the way ifs are constructed, but the framework-specific code that's in between.

No shims or polyfills are necessary for Array.methods, as our browser support is IE9+ / latest FF / latest Chrome.

If you have any patience remaining, I want to bring up something I must have missed before. As an example:

me.append(template, {
    'i': i,
    'item': item
});

We don't quote an object key unless necessary, e.g.:

me.append(template, {
    i: i,
    item: item
});

This occurs other places in the code as well (including index.html), I just used the above snippet as an example.

I ran into a console error while editing, then saving a blank todo:

Uncaught Error: NotFoundError: DOM Exception 8 jquery.js:5510
jQuery.fn.extend.remove jquery.js:5510
onDestroyClick list.js:151
handlerProxy maxi.js:3875
jQuery.event.dispatch jquery.js:4683
elemData.handle jquery.js:4367
jQuery.event.trigger jquery.js:4601
(anonymous function) jquery.js:5126
jQuery.extend.each jquery.js:590
jQuery.fn.jQuery.each jquery.js:237
jQuery.fn.extend.trigger jquery.js:5125
jQuery.fn.(anonymous function) jquery.js:6734
onEditFocusOut list.js:227
handlerProxy maxi.js:3875
jQuery.event.dispatch jquery.js:4683
elemData.handle jquery.js:4367
jQuery.event.trigger jquery.js:4601
(anonymous function) jquery.js:5126
jQuery.extend.each jquery.js:590
jQuery.fn.jQuery.each jquery.js:237
jQuery.fn.extend.trigger jquery.js:5125
jQuery.fn.(anonymous function) jquery.js:6734
onEditKeyUp list.js:203
handlerProxy maxi.js:3875
jQuery.event.dispatch jquery.js:4683
elemData.handle jquery.js:4367

And lastly, please squash your commits down to just 1. We'll take a final look and give @passy a chance to weigh in.

[mikaelkaron]

So I remember I had this problem before, and it looks like others have to.

I'll get right on fixing it.

[mikaelkaron]

All done, I guess we're waiting for @passy ;)

[stephenplusplus]

Thanks for all of the tweaking, styling, and rebasing!

I just updated, and ran into an issue. Trying to edit an item twice leaves the input field disabled, allowing me to bring up the input, but not do anything with it. To reproduce:

1. Create new todo.
2. Double click to edit todo.
3. Enter a new value.
4. Enter to save.
5. Double click to edit todo.

Also, the toggle all checkbox seems to go out of sync with the page in a particular scenario. To reproduce:

1. Start fresh with no todos.
2. Create a new todo.
3. Mark as complete.
4. Refresh the page.
5. Double click to edit todo.
6. Enter a new value.
7. Enter to save.
8. Mark the item as incomplete.

The check all box should remain in the "all are checked" state, however the item will appear as active. In this state, removing the todo item will remove it from the list, however it will be there if you refresh the page.

[mikaelkaron]

So I think this is because a 'broken' fix for chrome. Can you reproduce after 46186c9 (not that this brings back the chrome bug, I'll have fix that in a different way)

[mikaelkaron]

Think I have a better chrome fix. Updated and squashed. Thanks btw for doing all of this QA on the app for me.

[stephenplusplus]

Looks pretty solid to me! @passy, any last words?

[passy]

Awesome. I'll check it out when I'm back home from my cocktail tour.
On May 29, 2013 9:11 PM, "Stephen Sawchuk" notifications@github.com wrote:

Looks pretty solid to me! @passy https://github.com/passy, any last
words?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/575#issuecomment-18639155
.

[passy]

Fantastic work! Thank you, @mikaelkaron. 🍰

Also thanks for reviewing, @stephenplusplus! :)

[mikaelkaron]

Thanks for merging @passy. I have two last questions.

On the main site it says

• Maroon = App requires further work to be spec-compliant

Do I need to make any more changes to be spec compliant, or is the link to TroopJS not maroon?

At the same time I would like to ask how a project graduates from labs?

[passy]

@mikaelkaron The labs status includes a few factors. One of them is spec compliance, which TroopJS meets, but there are also external factors like user adoption, real-world usage and an active community among other things that we like to see in a framework before we graduate it from labs. I hope this helps. :)

[sindresorhus]

Nice work guys :)

@mikaelkaron we usually decide that before a TodoMVC release.

[mikaelkaron]

Ok. That sheds some light on it. But before that, do you think I could have the "maroon" part dropped as we're spec compliant?