A GWT version of the JavaScript TodoMVC sample application

144 views
Skip to first unread message

Colin Eberhardt

unread,
Mar 12, 2012, 1:24:26 PM3/12/12
to Google Web Toolkit
This might be of interest to members of this group ...

I have contributed a GWT version to the JavaScript TodoMVC project:

http://www.scottlogic.co.uk/blog/colin/2012/03/developing-a-gwt-todomvc-application/

For those of you who have not come across this project, it provides an
implementation of the same simple todo application with a wide range
of JavaScript frameworks (18 at the moment). I noticed it was lacking
a GWT implementation, hence my contribution, which has been reviewed
and accepted :-)

Here is the project homepage

http://addyosmani.github.com/todomvc/

And my GWT implementation in action here:

http://colineberhardt.github.com/

Regards, Colin E.

Thomas Broyer

unread,
Mar 12, 2012, 2:03:13 PM3/12/12
to google-we...@googlegroups.com
A few remarks:
  • I would have passed the task's text to addTask rather than having a getTaskText() on the view (i.e. make the presenter manage the state, and the view only display it and route user events to the presenter)
  • Instead of wrapping everything in a UiBinder, I would have kept part of the template in the HTML host page (I'd have wrapped only the <div id=todoapp>, keeping the static instructions and credits sections in the HTML host page)
  • I would have called the ViewEventHandler interface 'Presenter' (as many people do it), and use a setter on the view (setPresenter) rather than an observer-like method (addViewEventHandler)
  • I wonder if you could have used a CompositeCell to split the "done checkbox" and "delete button" handling in their own cells
  • I really don't like having that delete() method in the ToDoItem class, that's something that should go in the Presenter (you're doing MVP, not MVC). The setDone should IMO also go through the presenter as that one needs to be notified (the presenter would then call setDone on the model); there's no need to do it for setTitle(). The fact that the model knows the presenter is a sign that this is not actually a "model" but rather a "view model" (MVVM).
  • I don't understand you 'data-timestamp' hack in the Cell; it looks to me like you should have used ViewData and rely on the existing AbstractEditableCell behavior, but there might be reasons you didn't do it, and/or I might be wrong.
  • I would have used AutoBeans for the JSON serialization, instead of JSONParser/JSONValue and the like (which are a PITA to use)
  • I would have used Guava for the various string and list processing
  • Hiding the footer should be controlled by the presenter, not hidden in the view, IMO (easier to unit-test if you ask me; btw, you didn't unit-test your presenter)
Message has been deleted

kritic

unread,
Mar 12, 2012, 5:00:15 PM3/12/12
to google-we...@googlegroups.com
Must have been a lot of work getting this done. Thank you very much. It's a huge help with making things a little more clear. Also, the other implementations with Backbone etc. are very, very helpful. 

Colin Eberhardt

unread,
Mar 12, 2012, 5:40:06 PM3/12/12
to Google Web Toolkit
Thanks for the detailed comments ... a few responses:

>    - I would have passed the task's text to addTask rather than having a
>    getTaskText() on the view (i.e. make the presenter manage the state, and
>    the view only display it and route user events to the presenter)

I like this suggestion - will make this changed.

>    - Instead of wrapping everything in a UiBinder, I would have kept part
>    of the template in the HTML host page (I'd have wrapped only the <div
>    id=todoapp>, keeping the static instructions and credits sections in the
>    HTML host page)

I'm not so keen on this based on the goal of this project, to compare
and contrast it with various JavaScript frameworks. Having the markup
in a single place makes this comparison easier.

>    - I would have called the ViewEventHandler interface 'Presenter' (as
>    many people do it), and use a setter on the view (setPresenter) rather than
>    an observer-like method (addViewEventHandler)

I'm not so keen on that naming, having an interface for the Presenter,
which is a restricted view onto the presenter for the purposes of
responding to 'events' from the view seems wrong to me.

>    - I wonder if you could have used a CompositeCell to split the "done
>    checkbox" and "delete button" handling in their own cells

I thought about that too, but one of the goals of the TodoMVC project
is to use the same HTML markup and CSS for all the implementations.
Mine already deviates due to CellList using divs rather than ul / li
(it's a great shame this cannot be configured), so I wanted to adhere
to the template as much as I could elsewhere.

>    - I really don't like having that delete() method in the ToDoItem class,
>    that's something that should go in the Presenter (you're doing MVP, not
>    MVC). The setDone should IMO also go through the presenter as that one
>    needs to be notified (the presenter would then call setDone on the model);
>    there's no need to do it for setTitle(). The fact that the model knows the
>    presenter is a sign that this is not actually a "model" but rather a "view
>    model" (MVVM).

I see your point, this is a little more MVVM-like (considering I have
been working with Silverlight / WPF / Knockout for quite some time,
this is not surprising). I'll think on this some more.

>    - I don't understand you 'data-timestamp' hack in the Cell; it looks to
>    me like you should have used ViewData and rely on the existing
>    AbstractEditableCell behavior, but there might be reasons you didn't do it,
>    and/or I might be wrong.

I did try using AbstractEditableCell previously, but hit an issue
which I cannot recall. I'll revisit this.

>    - I would have used AutoBeans for the JSON serialization, instead of
>    JSONParser/JSONValue and the like (which are a PITA to use)

Good idea - I have been meaning to give AutoBeans a go.

>    - I would have used Guava for the various string and list processing

Feels a bit like overkill for such a small sample application.

>    - Hiding the footer should be controlled by the presenter, not hidden in
>    the view, IMO (easier to unit-test if you ask me; btw, you didn't unit-test
>    your presenter)

Personally I am not averse to placing a very small amount of
presentation-oriented logic into the view, specifically the sort of
logic which is so trivial that I would skip unit testing it.

And, yes, I haven't unit tested - this is in common with the other
TodoGWT projects, none have unit tests, where many are of course
perfectly testable.

Colin E.

Colin Eberhardt

unread,
Mar 12, 2012, 5:40:44 PM3/12/12
to Google Web Toolkit
Thanks, glad you like it. The whole TodoMVC project is a great idea in
my opinion.

Thomas Broyer

unread,
Mar 13, 2012, 5:52:56 AM3/13/12
to google-we...@googlegroups.com


On Monday, March 12, 2012 10:40:06 PM UTC+1, Colin Eberhardt wrote:
>    - Instead of wrapping everything in a UiBinder, I would have kept part
>    of the template in the HTML host page (I'd have wrapped only the <div
>    id=todoapp>, keeping the static instructions and credits sections in the
>    HTML host page)

I'm not so keen on this based on the goal of this project, to compare
and contrast it with various JavaScript frameworks. Having the markup
in a single place makes this comparison easier.

On the other hand, people not knowing GWT could think it *cannot* simply enhance an existing HTML layout, as the other implementations do. I see TodoMVC as a way of "advertizing" toolkits/frameworks, and I fear people see GWT as *only* being capable of building "empty window" interfaces [http://bitworking.org/news/427/js-rest-and-empty-windows]

We/you could go even farther as to putting almost everything in the HTML host page, using TextBox.wrap() to handle events on the text box, and RootPanel.get(xxx) to add the CellList and footer.
 
>    - I would have called the ViewEventHandler interface 'Presenter' (as
>    many people do it), and use a setter on the view (setPresenter) rather than
>    an observer-like method (addViewEventHandler)

I'm not so keen on that naming, having an interface for the Presenter,
which is a restricted view onto the presenter for the purposes of
responding to 'events' from the view seems wrong to me.

Well, that's exactly what the ViewEventHandler interface is; you're not forced to use an interface (the view could know the presenter) but helps in decoupling. You're technically allowing multiple handlers to be attached to the same view, but that's not going to happen in practice, or so rarely that it's not worth the runtime cost.
 
>    - I wonder if you could have used a CompositeCell to split the "done
>    checkbox" and "delete button" handling in their own cells

I thought about that too, but one of the goals of the TodoMVC project
is to use the same HTML markup and CSS for all the implementations.
Mine already deviates due to CellList using divs rather than ul / li
(it's a great shame this cannot be configured), so I wanted to adhere
to the template as much as I could elsewhere.

The stated goal is that the "look and feel" the same.
Also, there's nothing wrong not using ul / li provided you communicate the role of elements by other means (ARIA). Maybe CellList should be enhanced with ARIA for better accessibility, I haven't checked, but if that's the case, it's an issue with GWT that I don't think the TodoMVC sample should try to address (at least not at all cost). In trunk, CellTable now has a Builder so I believe you could make one using <ul> as the container, <li> for rows and <span> for cells.
Of course, YMMV.

If you look at the Ext.js (for instance), it doesn't use ul / li either, and its use of ARIA adds nothing to accessibility (role=presentation everywhere).

 
>    - I would have used AutoBeans for the JSON serialization, instead of
>    JSONParser/JSONValue and the like (which are a PITA to use)

Good idea - I have been meaning to give AutoBeans a go.

Note that because AutoBeans are only about data, you'll be forced to move away from the MVVM-like approach before using AutoBean (unless you use an AutoBean to wrap a ToDoItem, but that'd complicate things for little-to-no added value)

Colin Eberhardt

unread,
Mar 13, 2012, 7:09:17 AM3/13/12
to Google Web Toolkit
On Mar 13, 9:52 am, Thomas Broyer <t.bro...@gmail.com> wrote:
> On Monday, March 12, 2012 10:40:06 PM UTC+1, Colin Eberhardt wrote:
>
> > >    - Instead of wrapping everything in a UiBinder, I would have kept
> > part
> > >    of the template in the HTML host page (I'd have wrapped only the <div
> > >    id=todoapp>, keeping the static instructions and credits sections in
> > the
> > >    HTML host page)
>
> > I'm not so keen on this based on the goal of this project, to compare
> > and contrast it with various JavaScript frameworks. Having the markup
> > in a single place makes this comparison easier.
>
> On the other hand, people not knowing GWT could think it *cannot* simply
> enhance an existing HTML layout, as the other implementations do. I see
> TodoMVC as a way of "advertizing" toolkits/frameworks, and I fear people
> see GWT as *only* being capable of building "empty window" interfaces [http://bitworking.org/news/427/js-rest-and-empty-windows]

It is hard to second-guess the misconceptions a JavaScript developer
may or may not have about GWT. It is such a different programming
model.


> > >    - I wonder if you could have used a CompositeCell to split the "done
> > >    checkbox" and "delete button" handling in their own cells
>
> > I thought about that too, but one of the goals of the TodoMVC project
> > is to use the same HTML markup and CSS for all the implementations.
> > Mine already deviates due to CellList using divs rather than ul / li
> > (it's a great shame this cannot be configured), so I wanted to adhere
> > to the template as much as I could elsewhere.
>
> I don't see such a "goal" stated inhttps://github.com/addyosmani/todomvc/wiki/Todo-Application-Specifica...
> The stated goal is that the "look and feel" the same.
> Also, there's nothing wrong not using ul / li provided you communicate the
> role of elements by other means (ARIA). Maybe CellList should be enhanced
> with ARIA for better accessibility, I haven't checked, but if that's the
> case, it's an issue with GWT that I don't think the TodoMVC sample should
> try to address (at least not at all cost). In trunk, CellTable now has a
> Builder so I believe you could make one using <ul> as the container, <li>
> for rows and <span> for cells.
> Of course, YMMV.

This is a stated goal of the project, albeit a recent one. When I
submitted my GWT implementation there was a quite thorough code
review, where deviations from the supplied HTML template were flagged
and frowned upon!

I totally agree that there is nothing wrong with an alternative markup
- however, I wanted to be forgiving to the team that run the project
and accommodating to their requests.

As I mentioned, I will take your comments on board and appreciate the
feedback.

You are of course welcome to make changes and add pull requests for
updates to this code, as is anyone else.

Regards, Colin E.

Reply all
Reply to author
Forward
0 new messages