ruby-hyperloop / hyper-react

The project has moved to Hyperstack!!
https://hyperstack.org/
MIT License
285 stars 14 forks source link

while_loading / on_error redo #251

Open catmando opened 6 years ago

catmando commented 6 years ago

currently .while_loading is attached to a component like this:

Component(p1: exp1) do
  ... render some stuff
end
.while_loading do
 ... render this stuff if loading
end

Now that React 16 (and Hyperloop 1.0) have error handling it would be nice to extend this syntax like this:

Component(p1: exp1) do
  ...
end
.while_loading do
  ...
end
.rescue do # yes handily enough that is legal syntax
  ...
end

but in order for this to work there is a vast amount of trickery inside of Hyper-React and Hyper-Mesh.

Everything can be greatly simplified if we create a wrapper Psuedo component, and just reuse the existing event handler mechanism:

Try do
  ....render some stuff
end
.on(:loading) do
  ...replace with this if loading 
.on(:exception) do |e|
  ...replace with this if an error occurs (or re-raise e)
end
    # user is an active record model instance
    ... LI { Try { user.name }.on(:loading) { IMG {src: 'spinner.gif' } } }
  Try { DisplayThing(thing: some_thing) }
  .on(:exception) { |e| DIV { "some thing is wrong" } } 

I am proposing using the .on(...) syntax for a couple of reasons:

  1. less syntax to learn
  2. everything can be explained in terms of existing constructs
  3. everything can be built on top of existing hyper-loop without messing with internals

Note that the current 1.0.0 (edge) error handling call back (as defined by @janbiedermann ) would remain and can still be used, and would in fact be the basis for implementing the above.

janbiedermann commented 6 years ago

i still dont like it its out of place, not logic that .on replaces a render block, .on doesn't do such things

barriehadfield commented 6 years ago

Sorry to be wishy-washy, but I honestly don't mind either way. .on implies its an event handler so we are extending that idea a bit, but it looks OK to me either way

sfcgeorge commented 6 years ago

I also think they seem similar and I'm not too bothered. They kinda seem like events, like JS onload and such.

Is there also an while_loading inside the component as a hook? I see 2 things you might want to do. Using someone else's component you do your own stuff while loading using the events above. But if you're making a component then within that component you might want to show a placeholder while the data is loading. Something like:

class Foo < Hyperloop::Component
  # I guess this is used instead of render until render is done
  while_loading do
    DIV { "loading foos" }
  end

  render(DIV) do
    ::Foo.all.each do |foo|
      DIV { foo.name }
    end
  end
end

And a 3rd option, you may wish to display the load indicator inline:

class Foo < Hyperloop::Component
  render(DIV) do
    unless ReactiveRecord::WhileLoading.quiet?
      DIV(class: "load-indicator") { "Loading..." }
    end

    ::Foo.all.each do |foo|
      DIV { foo.name }
    end
  end
end
catmando commented 6 years ago

second option, yeah can be done, or also perhaps:

render(DIV) do
end
.on(:loading) do
...
end

the callbacks are nice too, though or you can always just do this:

render do
  Try do
    ...
  end
  .on(:loading) do
  ...
  end
end

Regarding the third option... I don't quite understand it... do you mean

if ...quiet?
   normal code
else
  code to show while loading?
end

that is okay but Try / while_loading will give you a lot more power. In particular it will only react to the code within its block, while quiet? is global. Also Try / while_loading can be nested. so if deep down you do a load and its guarded by a Try / while_loading then outer components will not be effected.

The idea is deep down I am getting data on say a user in a list... that list item can have its own spinner and the whole page does not have to effected (if that is your desired behavior)