Skip to content

Conversation

Geoffrey-D
Copy link

This facilitate DOM manipulation :)

@N00ts
Copy link
Owner

N00ts commented Jul 23, 2022

Hello, since vue js is based on data mutation for DOM manipulation (virutal DOM), i'm not sure having an ID for manipulation is relevant. Why did you put "data-node-id" instead of a real :id ? What is your use case ?

@Geoffrey-D
Copy link
Author

Geoffrey-D commented Jul 23, 2022

I need to open and focus nodes programatically, and as no "API" allowed such things, I had to:

  • Modifying the tree props to set all parents as "opened"

  • Manipulating the DOM to manage the "focus" state (by toggling classes).

I have added in this MR 2 API methods to do those things in an "API" object:

this.$refs.Tree.API.open(id);
this.$refs.Tree.API.focus(id);

I have also changed the "data-node-id" attr to "id" as it makes more sense

@Geoffrey-D
Copy link
Author

What do you think of those 2 features? Can we merge them?

@Geoffrey-D
Copy link
Author

Hey @N00ts could you please give a feedback on this?

Thx :)

@N00ts
Copy link
Owner

N00ts commented Aug 16, 2022

Hey I had a look to your PR. I have multiple remarks.

  • Maybe just returning state in useTree would allow you to do whatever API you like (can $ref.tree.state in parent component).
  • If you really need those API, maybe just put them directly at root func of Tree (instead of tree.API.method). Can you also add unit tests (there is a yarn test) ?
  • I saw that dev.vue was modified, can you revert it ?

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