Closure app - new template #155

Closed
sindresorhus opened this Issue May 1, 2012 · 8 comments

Comments

Projects
None yet
2 participants
Owner

sindresorhus commented May 1, 2012

Closure app needs to be updated to the newest template.

  • Upgrade to latest Closure library
  • Use the latest template.
  • Make sure you follow the newly released App Specification.
    The spine.js app can be used as a reference.
  • This project uses tab indention and follows the Idiomatic guide to writing JavaScript. Please ensure your code follows this closely. If something is unclear, follow the existing style. Use double-quotes in HTML and single-quotes in JS and CSS.
Owner

sindresorhus commented May 1, 2012

@chrisprice Since you created the original app, would you be interested in updating it?

Member

chrisprice commented May 1, 2012

Sure, I'll take a look

Member

chrisprice commented May 13, 2012

Howdy, I think I'm getting there but I've got a couple of queries -

The template has a lowercase "todos" [1] but the Spine.js example has a titlecase "Todos" [2], which should I be copying?

The Closure toolkit contains a linter [3] which I'd really like to use fully, but it doesn't support configuration of tabs/spaces [4]. Currently I'm ignoring the whitespace warnings but it's not ideal, would it be possible to use spaces for this example? (I'm not trying to start a religious war here, it would just be nice to make use of the official tools, I'm happy to go with whatever)

I'm yet to implement the routes support because I'm not sure whether the library would be considered to support it or not - where are you drawing the line?

I'm liking the new template by the way, but the destroy css animation seems to intermittently lag chrome up, have you noticed that? It might be because I've got the FPS counter on...

[1] https://github.com/addyosmani/todomvc/blob/master/template/index.html#L17
[2] https://github.com/addyosmani/todomvc/blob/master/architecture-examples/spine/index.html#L15
[3] https://developers.google.com/closure/utilities/
[4] https://groups.google.com/forum/?fromgroups#!topic/closure-linter-discuss/yCvu-v-hdi8

Owner

sindresorhus commented May 13, 2012

It's todos

Sure, use spaces, but be consistent, and only in the JS code, tabs in HTML.

I'm not sure where to draw the line, it's mostly a judgement call. Can you refer me to the docs about routing in Closure?

It's silky smooth on both my Mac's. What OS are you running? It might be that your graphic card is not used for the animation, that will probably have some impact.

Can you also add a readme.md file with a notice about the use of space and why, your name, and instruction on how to use/build? See the readme in the spine app.

Member

chrisprice commented May 14, 2012

Cool, I'll make those changes.

Well searching for route in trunk doesn't reveal much [1], nor does routing [2], I think the closest thing is the URL-fragment support in History [3]. I'm happy to build something around that because I think it will show the general structure and which classes would be involved in such an implementation. Would that be ok?

I'm on a different PC at the mo, but it was Win7 and the hardware is relatively new so my guess is a software (though most likely not css) problem. I'll see if I can reproduce it tonight.

I'll stick a readme in there, currently there's some instructions in comments at the end of the HTML file but a readme seems a far more appropriate location!

[1] http://code.google.com/p/closure-library/source/search?q=route&origq=route&btnG=Search+Trunk
[2] http://code.google.com/p/closure-library/source/search?q=routing&origq=routing&btnG=Search+Trunk
[3] http://code.google.com/p/closure-library/source/search?q=history&origq=history&btnG=Search+Trunk

Owner

sindresorhus commented May 14, 2012

Yes, it seems to have a History class, which although not routing directly, is still more than nothing. Go for it ;)

Member

chrisprice commented May 14, 2012

Routing still pending but the other changes are done.

Still not tried on my PC but I can reproduce a similar glitch in Chrome 18.0.1025.168 on OSX 10.6.8 by slowly rolling back and forth over the destroy button a few times on the spine.js example. There's a full screen black flash every now and again. Once it starts happening I can reliably reproduce by mouse over until the animation completes then mouse out. Weird!

Member

chrisprice commented May 19, 2012

Routing is in and I've submitted the pull request #172.

I've also noticed a minor cosmetic bug. When the clear completed button is toggled it causes all of the text in the status bar to jump. Adding height:20px to the #footer seems to fix it.

@gustaff-weldon gustaff-weldon pushed a commit to gustaff-weldon/todomvc that referenced this issue Dec 23, 2013

@chrisprice @sindresorhus chrisprice + sindresorhus Closes #172: Closure app update. Fixes #155 9ad986a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment