Added a bunch of jQuery javascript helpers and more...

[kristianmandrup]

Made it more compatible with asset pipeline and improved generator placement structure

[kuraga]

Ok, I have seen half of commits. And I saw two incompatibilities: views are moved to views subfolder and display default state is renamed to show... Your opinion, Nick?
UPD: It seems that situation of an old behavior are handled, too.

[apotonick]

Could we have separate discussions about the (upcoming?) views/ subfolder for cells and widgets and the jQuery helpers?

[kuraga]

Yes, sure. Do you mean new github issue?

[kuraga]

@kristianmandrup Don't you remember some (else) incompatibilities introduced by this PR?

[apotonick]

It would be great if this PR gets split into features that can be merged separately as I don't have the time to go through all the commits. We don't need to open more PRs, just a mapping from commits to feature.

[kuraga]

No. These things are not separated by Nick (i.e. see the first commit). By think that I can split each diff... And squash up then. But I need time for this.

[kristianmandrup]

To me it makes perfect sense for the default action to be named #show to be in line with Rails REST conventions.
Perhaps there should be an option to somehow indicate that a widget can handle both a collection and simple items via #index and #show ? Do widgets even need views in the form of templates in the traditional Rails sense?

One could argue that the best practice would be to partition the interface into sufficiently small parts, that each "view" could better be handled by a Decorator or similar and take advantage of inheritance etc for better reuse and encapsulation of business logic? Just some thoughts... I would think, that ideally the templates should only be used as Proof of Concept, before refactoring them into decorators? At least, that would be my choice.

[apotonick]

@kuraga Wait, first I need an overview about which features are in the PR. I can see jQuery extensions right away.

@kristianmandrup The default state of a widget should be #show as #display is an internal ruby method. Also, I agree, it matches with Rails' CRUD convention.

What I don't understand is that decorator thing? Do you mean to decorate the widget, or the models contained in the widget?

[kristianmandrup]

What I mean is that if you have a widget, which encapsulates a small view part, it can become a mess with all these mini view templates IMO. I feel that a better approach is to have the view rendered directly via a Decorator class, which you can test independently, something like Draper for example. Then it could look sth like:

render text: decorator.show(self)

I would imagine?

class MyWidget < Widget
  def show
    render text: decorator.show(self)
  end

  protected

  def decorator widget
    MyWidgetDecorator.new(widget).show
  end
end

A "CRUDe" example ;) but the basic idea... The Decorator then gets all the power of OOP, such as inheritance, polymorphism, modularization etc. I think it is a small price to pay for the little extra complexity. The view templates could of course still be used in the quick Proof of Concept (prototype) phase. Just my thoughts...

[kuraga]

People let's separate discussions! The age of this PR is half-of-year. We should step forward...

[apotonick]

Let's move on with the new assets layout where we add the views
directory. This should go into cells!

[kristianmandrup]

I didn't test all these helpers. Just started adding whichever I thought might be useful... not even sure this is the right approach :P Perhaps there is a better way, without having ruby "spit out" javascript? Perhaps better with more clever use of Object Oriented coffeescript? Maybe have a few predefined coffeescript functions that handle this instead?

[kuraga]

@kristianmandrup example please?

[kristianmandrup]

Imagine we have an Apotomo.Widget object in coffeescript:

class Apotomo.Widget
  initialize(id) -> 
    @id = id

  execute(method, *args)  ->
    # dynamically execute jquery function on widget (via @id)

  # or just have each function like this...
  toggleClass(name ->
    $(id).toggle(name)

Putting the functionality where it "belongs" ;) no?

[kuraga]

@kristianmandrup Do you say about functionality which is in commits now? If yes then I think that is good have both functionalities... Your example is very nice but what if I don't want create some special JS objects?...

But about helpers... Should they be JS-agnostic maybe? Without jQuery?

[kristianmandrup]

I would add jQuery helpers, since this is the new default as I understand since Rails 3.1. If some developers want to add equivalent helpers for other javascript frameworks or pure javascript, they may do so... leave room for others ;)
Should just be "proof of concept" to target most common usage, others may expand this if they need.

[kuraga]

Yes, I agree. Oh, I see! Frameworks are separated by modules.

[kuraga]

Ok, @apotonick @kristianmandrup I think I should split these commits and analyze them. I will do this. For own experience or for Apotomo or for my project, I don't care... I can say nothing about time... And sure you can do this work parallel with/out me. Thanks.

[kristianmandrup]

Look at this :)

class Apotomo.WidgetRepo
  initialize ->
    @widgets = []

  find(id) ->
    @widgets[id]

  register(widget) ->
    @widgets[widget.id] = widget

Apotomo.widgets = new Apotomo.WidgetRepo

class Apotomo.Widget
  initialize id ->
    @id = id
    Apotomo.widgets.register(self) # or this ?

  element ->
    $(id)

  toggleClass(name) -> 
    element.toggleClass(name)

  hide ->
    element.hide

class Property.TabWidget extends Apotomo.Widget
  initialize(id) ->
    super(id)

  show ->
    # ...

  collapse ->

  expand ->

PS: Above syntax may not be correct, but should make it clear what I mean

in ruby Widget class render method

class Property::TabWidget < Apotomo::Widget

  def render
    # some ruby code... fx operations on instance vars

    # some ruby expression to render the following in javascript
    # Apotomo.widgets.find(id).toggleClass('highlight`)

     # for example
     js_for(widget_id) do |widget|
       widget.toggleClass(highlight)
     end
  end

I think this approach would be way better, and nice encapsulation of functionality at both ends.

[kuraga]

Yes, sure, but I think it's not bad to have append method if we have replace method! Which we do have now; or do you suggest to remove JavascriptGenerators module at all?

[kristianmandrup]

I suggest to replace the ruby side with something like this:

My own widget

     client_widget.toggleClass(highlight)

Operate on some other widget by id

     client_widget(widget_id) do |widget|
       widget.toggleClass(highlight)
     end

or operate on multiple widgets

     client_widgets(id1, id2) do |widgets|
       widgets.each {|widget| widget.toggleClass(highlight) }
     end

In each case, rendering javascript like this (to retrieve the Javascript widget class instance from the Javascript widget repository)

Apotomo.widgets.find(id)

Nice and elegant :)

The JavaScriptGenerator then have to be refactored to support this new style... that's all.

[kuraga]

Hm... I thought that you suggest to introduce execute helper (with some code as argument) and remove other helpers...

Yes! I understand now! Very cool! @apotonick, Nick, your opinion?

P.S. Maybe nice to delegate toggleClass to client_widget automatically :)

[kristianmandrup]

Well, execute was meant as a more generic helper.

widget.execute(:highlight, 'orange') would render javascript such as:

widget.execute('highlight', orange') which would execute something like the following (coffeescript):

  execute(method, *args) ->
    $(id)[method](*args)

I think execute could be useful in addition to the other helpers (at least for prototyping functionality), but preferably the developer should populate the Javascript "twin class" for each widget with whatever specific methods it needs, and not rely on such a generic function. What do you think?

P.S: I don't understand what you mean by "delegate toggleClass to client_widget automatically" ? toggleClass was just as an example.

[kuraga]

I think that my experience in thinking of patterns is too small for this :) Let's wait for Nick's opinion.

One that I think - I think that helpers should be or not be. Or a comprehensive set of them, or one helper render only.

P.S. toggleClass(:highlight) may be equivalent to client_widget.toggleClass(:highligt).

[kristianmandrup]

ahh, but that is dangerous in my opinion. It is not clear in that case that toggleClass is applied on the client side widget and rendered as javascript. Too much magic! Why I introduced the block syntax, in order to make it clear that whatever was inside the block scope is rendered as javascript ;)

[kuraga]

Yeah, yeah... That's a pattern too. Will you, @kristianmandrup, code something two nearest weeks?

[kristianmandrup]

Yeah, if you get started by taking the best of my ideas and cleaning up the code, I will be ready to help out from next week or this weekend ;)

[kuraga]

I'll start by splitting commits only (and reading them). Then'll talk ;)

[kristianmandrup]

Skype!?