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

Return callback for pre walk #58

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

krzysztof-grzybek
Copy link

@krzysztof-grzybek krzysztof-grzybek commented Jul 27, 2018

Related to #36

@coveralls
Copy link

coveralls commented Jul 27, 2018

Coverage Status

Coverage increased (+0.09%) to 98.889% when pulling f36d330 on krzysztof-grzybek:return-callback-for-pre-walk into 1ac126a on joaonuno:master.

@joaonuno
Copy link
Owner

Hi, thank you for contributing! A few questions:

  1. Can you give an example on the use-cases for this feature?
  2. Is it only useful in depth-first pre-order strategy?
  3. How is this related to issue 22?

@krzysztof-grzybek
Copy link
Author

Hi Joanuno,
thank You for quick response.

  1. I can describe my case which I'm struggling with:

I have layout data represented in Tree. I walk down the Tree and I add elements positioned absolute to the DOM. I keep top position in global variable. If node is group type, I insert child nodes inside (here top does not change). But after inserting all child nodes into the group, I have to increase my top value to render next sibling element below the group. The only thing comes to my mind is to increase it on hook after all child nodes has been visited.

If my explantion isn't clear let me know, I'll try to do it better :)

  1. To be honest I didn't think about it, but probably it can be useful for all strategies.
  2. Of course I linked wrong issue, sorry for that, it should issue Return callback for pre walk #36

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

Successfully merging this pull request may close these issues.

3 participants