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

Add jsblocks #1297

Closed
wants to merge 2 commits into from
Closed

Add jsblocks #1297

wants to merge 2 commits into from

Conversation

astoilkov
Copy link
Contributor

jsblocks have been developed for the past couple of years. Two weeks ago it was released and collected more than 800 stars. You could take a look at some of the advantages of the project here. Some of the things include:

  • Performance
  • Debugging exprience
  • Server-side rendering

Test results:

TodoMVC - blocks
  When page is initially opened
    √ should focus on the todo input field
  No Todos
    √ should hide #main and #footer (109ms)
  New Todo
    √ should allow me to add todo items (459ms)
    √ should clear text input field when an item is added (288ms)
    √ should append new items to the bottom of the list (570ms)
    √ should trim text input (262ms)
    √ should show #main and #footer when items added (253ms)
  Mark all as completed
    √ should allow me to mark all items as completed (610ms)
    √ should allow me to clear the completion state of all items (687ms)
    √ complete all checkbox should update state when items are completed / cleared (739ms)
  Item
    √ should allow me to mark items as complete (575ms)
    √ should allow me to un-mark items as complete (585ms)
    √ should allow me to edit an item (942ms)
    √ should show the remove button on hover
  Editing
    √ should hide other controls when editing (672ms)
    √ should save edits on enter (867ms)
    √ should save edits on blur (942ms)
    √ should trim entered text (868ms)
    √ should remove the item if an empty text string was entered (800ms)
    √ should cancel edits on escape (939ms)
  Counter
    √ should display the current number of todo items (399ms)
  Clear completed button
    √ should display the correct text (571ms)
    √ should remove completed items when clicked (865ms)
    √ should be hidden when there are no items that are completed (615ms)
  Persistence
    √ should persist its data (854ms)
  Routing
    √ should allow me to display active items (633ms)
    √ should respect the back button (892ms)
    √ should allow me to display completed items (616ms)
    √ should allow me to display all items (776ms)
    √ should highlight the currently applied filter (678ms)


30 passing (3m)

@urrgur
Copy link

urrgur commented May 25, 2015

👎

@mauricio-frontend
Copy link

This is a good Idea to put this on TodoMVC, I test JSBlock and like of your performance.

indent_size = 2

[*.md]
trim_trailing_whitespace = false
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this file. We have our own code styling rules and those are in the root directory :) Same thing for .gitattributes :)

@arthurvr
Copy link
Member

  • Please add this example to learn.json and to the home page.
  • should the folder be named blocks or jsblocks?
  • Please commit all dependencies as seen in other examples.

Can you also fix the JSHint errors?

@astoilkov
Copy link
Contributor Author

No problem. I will fix everything. Can you just clarify what do you mean by adding the example to the home page? Which home page and where?

@arthurvr
Copy link
Member

@astoilkov To the home page of the website, index.html. If you have any other questions don't hesitate to ask. Thanks for working on this!

@astoilkov
Copy link
Contributor Author

I think I fixed all reported things. As for the name of the folder - I have removed js because the npm package is called blocks and it is obvious that is a JavaScript framework. However, if you suggest that it is better to add the js I will rename the folder to jsblocks.

@arthurvr
Copy link
Member

arthurvr commented Jun 6, 2015

I think I fixed all reported things. As for the name of the folder - I have removed js because the npm package is called blocks and it is obvious that is a JavaScript framework. However, if you suggest that it is better to add the js I will rename the folder to jsblocks.

Well, we're not talking about an extension here. What's the name of the framework? Blocks? Or jsblocks?


node_modules/todomvc-common/*
!node_modules/todomvc-common/base.css
!node_modules/todomvc-common/base.js
Copy link
Member

Choose a reason for hiding this comment

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

This gitignore looks okay but you forgot to actually add the files too.

@astoilkov
Copy link
Contributor Author

It's jsblocks. However, in jsdelivr and DefinitelyTyped the folders are called blocks. So its your call. I can't decide.

@@ -0,0 +1,59 @@
<!doctype html>
<html lang="en" data-framework="jsblocks">
Copy link
Member

Choose a reason for hiding this comment

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

this should be equal to the folder name

@arthurvr
Copy link
Member

arthurvr commented Jun 6, 2015

Looks like I'm able to add empty todos.

@arthurvr
Copy link
Member

arthurvr commented Jun 6, 2015

@astoilkov jsblocks, then.

},

clearCompleted: function () {
this.removeAll(function (todo) {
Copy link
Member

Choose a reason for hiding this comment

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

Might be just my feeling but removeAll is a confusing name if you only want to remove specific items.

@astoilkov
Copy link
Contributor Author

Ok. Fixed noted things. Thanks for being so patient. :)

@samccone
Copy link
Member

samccone commented Jun 6, 2015

@astoilkov mind squashing down into a single commit please?

<link rel="stylesheet" href="node_modules/todomvc-app-css/index.css">
</head>
<body>
<section id="todoapp" data-query="view(Todos)" class="todoapp">
Copy link
Member

Choose a reason for hiding this comment

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

Should we use the v2 app spec here (classes instead of IDs) ?

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 added the id because the tests were failing without it. I can remove them quickly if this is not a problem.

Copy link
Member

Choose a reason for hiding this comment

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

You can remove the id's. We're going to update the test suite shortly.

@astoilkov
Copy link
Contributor Author

@samccone When everything is perfect I will make it a single commit. Just one question. I will do it by creating a new pull request with the one commit is there any other way for making it only one commit?

@arthurvr
Copy link
Member

arthurvr commented Jun 7, 2015

When everything is perfect I will make it a single commit. Just one question. I will do it by creating a new pull request with the one commit is there any other way for making it only one commit?

Just squash them using a rebase and then force push this branch. If you've got troubles with that I'm happy to squash stuff for you while merging :)

@@ -278,6 +278,9 @@
<li class="routing">
<a href="examples/webrx/" data-source="http://webrxjs.org" data-content="WebRx is a Javascript MVVM-Framework that combines functional-reactive programming with declarative Data-Binding, Templating and Client-Side Routing. The framework is built on top of ReactiveX for Javascript (RxJs) which is a powerful set of libraries for processing and querying asynchronous data-streams that can originate from diverse sources such as Http-Requests, Input-Events, Timers and much more.">WebRx</a>
</li>
<li class="routing">
<a href="examples/blocks/" data-source="http://jsblocks.com" data-content="From simple user interfaces to complex single-page applications using faster, server-side rendered and easy to learn framework">jsblocks</a>
Copy link
Member

Choose a reason for hiding this comment

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

This link is now broken ;)

@arthurvr
Copy link
Member

arthurvr commented Jun 7, 2015

Looks like I'm able to add empty todos.

This is only partially fixed. I'm still able to edit an existing one with only whitespace.

@arthurvr
Copy link
Member

arthurvr commented Jun 7, 2015

Could you please add some comments to the code? Specifically to the parts which might confuse beginners. (#385)

@arthurvr
Copy link
Member

ping @astoilkov, was you able to publish a release?

@astoilkov
Copy link
Contributor Author

@arthurvr No. Sorry. I was on a holiday and returned yesterday. I am a little sick now but I will publish a release next week.

@arthurvr
Copy link
Member

Thanks.

@astoilkov
Copy link
Contributor Author

Sorry for the delay. Here are the updates. :)

@samccone
Copy link
Member

@astoilkov looks like we need a rebase 😉

@astoilkov
Copy link
Contributor Author

I am not expert on git. Is there any possibility someone else can do it? If no...can you explain me the required steps you want me to take.

@sindresorhus
Copy link
Member

@astoilkov
Copy link
Contributor Author

Thanks @sindresorhus . It's time I do this.

@arthurvr
Copy link
Member

ping :)

@astoilkov
Copy link
Contributor Author

Yep. I am in process of completing it. Hope will do this in the next days.

@samccone
Copy link
Member

awesome thanks @astoilkov

@samccone
Copy link
Member

Hey @astoilkov looks like you are still failing a few tests, have you had a moment to take a look?

@astoilkov
Copy link
Contributor Author

Yes. I found the problem. I will try fixing it without pushing to the todomvc repository. I will write back once done.

@samccone
Copy link
Member

great thanks a bunch @astoilkov

@astoilkov
Copy link
Contributor Author

Hi @samccone. The issue is fixed. However, I will need to bump up the jsblocks version. I will fix a few more issues, bump up the version and then update it here.

@samccone
Copy link
Member

samccone commented Oct 5, 2015

ok thanks for keeping us in the loop @astoilkov !!!

@samccone
Copy link
Member

Hey @astoilkov any word :)

@astoilkov
Copy link
Contributor Author

Yes. As I said I will release this week. Today we did the last bug fix that we wanted to include in the release. I will spend some time these days to make the release.

@samccone
Copy link
Member

awesome ✨

Just doing my weekly review of open PRs :) :)

@astoilkov
Copy link
Contributor Author

Ok. I am ready with the release. Now the only thing I should do is update the pull request adding the latest jsblocks version. I have a few questions though:

  • I would probably need to rebase again right? This is extremely hard for me so I ask an external person for this to help me so it could take some time.
  • I think after the last rebase we have wiped out some of the changes in the index.html and the jsblocks framework isn't there. Can you confirm that? If this is true I should add it again right?

@samccone
Copy link
Member

would probably need to rebase again right? This is extremely hard for me so I ask an external person for this to help me so it could take some time.

Yep, you will want to rebase, however there are no conflicts so it should be as simple as git fetch <remote name>

git rebase <remote name> master

then git push -f

If this is true I should add it again right?

Yep add it back in :)

@samccone
Copy link
Member

Hey guys, I am going to close the PR, just ping us when you push and think it is good to go. Thanks

@samccone samccone closed this Oct 16, 2015
@astoilkov
Copy link
Contributor Author

👍

@Kanaye
Copy link

Kanaye commented Oct 16, 2015

Hey guys. We just updated the Repo and think we are ready to go 😄.

[edit]:
Sorry seems I derped with squashing the repo while in a closed PR. Didn't know that behaviour of github.
Can you do something about that or do we have to open a new one.
/cc @samccone @arthurvr

@samccone
Copy link
Member

oh looks like it freaked out :( @Kanaye mind just opening a new one and linking it? Thanks!

@Kanaye Kanaye mentioned this pull request Oct 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants