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

Recursive forwardTo doesn't work exactly how I'd hoped #14

Closed
parsonsmatt opened this issue Mar 29, 2016 · 9 comments
Closed

Recursive forwardTo doesn't work exactly how I'd hoped #14

parsonsmatt opened this issue Mar 29, 2016 · 9 comments

Comments

@parsonsmatt
Copy link
Contributor

I'm working on a toy app with a recursive structure. Here's my action:

data Action
    = Child Int Action
    | UpdateWeight Int Number
    | UpdateScore (Score Number)
    | AddGrade
    | Undo
    | Redo

And an example on how I'm using it:

renderScore :: Array (Score Number) -> Html Action
renderScore gs = H.div # do
     H.button ! E.onClick (const AddGrade) # H.text "Add Grade"
     H.ul ## 
        forEachIndexed gs \i g ->
            H.li # (H.forwardTo (Child i) (viewGrade g))

When the UI gets nested, forwardTo uses the most recent function given. It'd be nice if it composed the functions instead.

@alexmingoia
Copy link
Owner

forwardTo uses the most recent function given

Sorry for being thick, but I'm not sure what you mean by this. Could you explain another way? The code you posted works, yes?

@parsonsmatt
Copy link
Contributor Author

If I have a structure like:

data Tree a = Tree a (Array (Tree a))

data Action = Remove | Child Int Action

then I can render that with the following:

view :: Tree String -> Html Action
view (Tree string subtrees) = ul # do
    li # do
        text string
        button ! onClick (const Remove) # text "X"
    li # ul ## forEachIndexed \i tree ->
        forwardTo (Child i) (view tree)  

So, if I've got a tree like:

example :: Tree String
example = Tree "Hello" [Tree "World" [], Tree "Party" [Tree "Adventure" [], Tree "Time" []]]

That should render to a list like:

  • Hello [X]
    • World [X]
    • Party [X]
      • Adventure [X]
      • Time [X]

When "Adventure" has the X button clicked, then it'd be great for that to call Child 1 (Child 0 Remove)). Right now it just calls Child 0 Remove.

@parsonsmatt
Copy link
Contributor Author

exports.mapActions = function (parentAction, html) {
  var childAction = html.props.puxParentAction;
  if (childAction) console.log(childAction, html);
  return React.cloneElement(html, {
    puxParentAction: childAction ? parentAction(childAction) : parentAction
  });
};

I put this console.log in and I'm not getting it to log anything. Will debug further

@alexmingoia
Copy link
Owner

If I understand you correctly, there's a bug in my FFI implementation since the type signature for forwardTo should allow this behavior?

The FFI implementation can be a bit cryptic, because instead of simply mapping over the actions, a reference to the action is saved on the tree for later use by render in src/Pux.js. This is a performance optimization to avoid additional traversals of the vtree, and the purpose of the puxParentAction prop. It's probably also the source of this issue.

@parsonsmatt
Copy link
Contributor Author

The code looks like it should be calling parent's parent's actions. I don't know why puxParentAction isn't available on the html parameter. I'll get a minimal example together.

@parsonsmatt
Copy link
Contributor Author

Ok, here's an example project: https://github.com/parsonsmatt/pux-repro

image

Clicking "Time" should delete "Time", but instead it deletes "World"

image

@alexmingoia
Copy link
Owner

Thanks for that! It helps a lot. For some reason it looks like html.props.puxParentAction isn't there even when html is a child view that should have been run through fowardTo...

@alexmingoia
Copy link
Owner

Good news! This is fixed in v2.0.1. Thanks for bringing this to my attention and creating that example repo.

@parsonsmatt
Copy link
Contributor Author

Awesome, thanks! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants