Code Monkey home page Code Monkey logo

Comments (7)

grapoza avatar grapoza commented on May 29, 2024

Hey Chris, I absolutely agree the parent component should never need to manage the lifecycle of the treeNodeSpec. In the short term, the shadow tree would probably be a valid workaround, but is really heavy for something as common as filtering. That's a huge ask from a component, and shouldn't be needed.

Just to make sure we're on the same page, when a new node is added to the initialModel prop it should get the treeNodeSpec data added (with default values and any supplied modelDefaults applied). It sounds like you're replacing the whole tree structure with a filtered tree, which as you noted breaks the tree's internals. When taking a look at that stackblitz with the Vue Devtools it seems like the nodes themselves actually do get all the expected data in the treeNodeSpec, but the tree's own initialModel never gets updated with those changes somehow. I wonder if this broke when moving to Vue 3 as part of 4.x or during the composition refactoring as part of 5.x?

I think there are two possible paths forward long-term:

  1. Figure out what's going on with the initialModel issue here, and mitigate it (with a fix or a reworking of the internals somehow)
  2. Use something like a new treeNodeSpec.state.hidden attribute to filter the top-level and child nodes. If the current internals take this into account when computing child lists then it can probably be done without having to think about keyboard navigation etc.

I'll see if I can put some more thought into this this weekend when I'm fresher. I'll try to keep this issue updated with any findings and any decision on a design. Thanks for submitting this one.

from vue-tree.

luckyrat avatar luckyrat commented on May 29, 2024

I couldn't get the shadow tree idea to work. I think that even when adding the same object that was previously extracted from the initialModel tree, that has somehow lost an internal association (maybe a Vue Proxy or something else you track within the component - I'm not familiar enough with either to be sure).

Thanks for the example links. I've not used that site before but think I have managed to create a fork of the 2nd example which you can access here: https://stackblitz.com/edit/vitejs-vite-mughet?file=src/App.vue

I've tweaked your replacement function to more closely match the algorithm I've used to perform the replacement - I don't see any difference in behaviour from the example you wrote but thought it might be useful to see it anyway (and I've not hooked up Vue dev tools to investigate any of this yet so perhaps some differences would become apparent there).

I also added a 2nd replacement function which lets us explore what happens when a new node is submitted with a full set of treeNodeSpec properties, since that is similar to what would be needed to make the shadow tree idea work.

It's interesting that popping and pushing in your first example works but a push in replaceTreeData() in my forked 2nd example does not.

I am replacing the entire tree, although as you can see in my forked example, trying to do so in a way that doesn't affect the object (Array) reference that was passed to the initialModel prop. I'm not sure how else I could approach it since any number of individual child items, or even all children from a particular node could need to be hidden or re-added each time the filter gets re-applied.

As long as (2) can correctly react to a change to that hidden attribute when applied from a parent component (and the rest of internals can take it into account in the way you already mention) it sounds like a decent approach to me - I could just walk the same tree that I supply to initialModel and update that state whenever I need to hide or show a node.

I guess that (1) might result in some mitigation that has knock-on benefits to other interactions with the component beyond just the need for filtering out nodes. Perhaps when reviewing the internals to make (2) work, an explanation of the behaviour in (1) will magically appear, or vice-versa.

from vue-tree.

grapoza avatar grapoza commented on May 29, 2024

Thanks for all the great feedback and investigation. I started a proof-of-concept for filtering yesterday and am working on it again today. I'm hoping to narrow it down to the point where you as a user don't have to care about setting things in the treeNodeSpec at all. My current thought (idea number three at this point, I think?) is to have a filter prop on the tree itself, which you can set and update with a filter function that works on a node and returns a boolean (true = show, false = hide). The tree supplies that to the nodes with provide/inject, and nodes use the filter function and the child nodes' state to compute whether they're hidden.

This would let you provide the filtering as :filter-prop-name="myFilterMethod" and even have a computed filter method that changes when your filter string changes (I'm assuming you're binding filtering to a text field or something, and this may be total overkill):

const myFilterMethod = computed(() => (node) => node.name === someFilterRef.value)

I'm not even sure that would work; I'm spitballing at this point. More likely, you'd probably update the filter method by watching your filter value, debouncing as needed, and then setting the filter method when the user pauses typing.

from vue-tree.

luckyrat avatar luckyrat commented on May 29, 2024

I've not created a function output from a computed value before but assuming that works reliably then this sounds like a powerful/flexible solution to me. You're right that I have a text field bound to the filtering term (which in practice would be more like a search for the presence of that substring than an equality check but the functional approach means that's not a detail you'd need to be concerned about).

I agree that the parent component should be responsible for debouncing, unless for some reason you have to handle it internally. I think it's better handled by the parent component but it's probably not a serious problem if it can't be done that way.

Thanks for the ongoing investigation. Let me know if you come up with any proof of concept code you think it would be worth me trying out.

I can't promise that this will be a simple code review but in case you are interested, the Vuetify team have previously supported a filter prop for Vue 2 and are changing it for Vue 3 ("filter prop removed. use props from filter composable", which doesn't mean much to me so I'm glad I'm not going to be updating to that if they ever finish the changes) - vuetifyjs/vuetify#14715 in case you want to dig into their source code for fun one day! They used to offer a simple string prop which when updated would cause the tree to filter based on a substring search of the label. So nowhere near as generally useful as your idea number 3 but it did used to be good enough for my current use case.

from vue-tree.

grapoza avatar grapoza commented on May 29, 2024

Alright, I've created a quick poc for filtering using the inject approach I described above (didn't touch the whole computed-as-a-param-to-the-treeview thing yet). If you grab the branch for the poc you can run the storybook task and try it out. The "Filtering" Storybook story has a text field and a button to apply the filtering, which will case-insensitively match the label text and display any matching nodes and their parents.

I haven't added support for recognizing filteredness to things like expansion/selection/traversal, but now that it's proven that I can filiter I don't think there's much stopping me from getting that stuff working.

There's some leftover stuff from previous attempts that I'll flesh out and keep around or split into a new story, like being able to do postorder depth-first traversals. You can ignore that if you take a look.

Thanks for the Veutify link, they do really cool stuff with the way they organize their composables and generalize the functionality. In a final solution the filtering for this component will hopefully get separated in a similar way, as I'll want to reuse what I can in the TreeGrid implementation I'm (slowly) working on.

from vue-tree.

grapoza avatar grapoza commented on May 29, 2024

Alright, so I've basically finished the branch to implement filtering with the exception of most unit tests and looking into corner cases (data changes that affect filtering reacting appropriately in all cases?) I'll get working on those, but if you want to take a look hopefully it'll meet your needs.

from vue-tree.

luckyrat avatar luckyrat commented on May 29, 2024

Sorry for not getting a chance to look at the branch until now. I've built the branch locally and tried to integrate it to a branch in my project (https://github.com/kee-org/browser-addon/tree/feature/save-group-filtering). Unfortunately it's not working yet, I'm not sure why and have run out of time for today (and possibly all week).

I've checked that my filterMethod is being called and confirmed it is returning true/false as it should but the tree itself never changes (well actually some CSS class might change judging by a slight padding shift I see when the filterMethod is not null but I suspect that is irrelevant).

I've skimmed through the code changes in your branch and although my knowledge of the Vue composition API is pretty weak, it looked like a decent approach and nothing obviously wrong jumped out at me.

I'm not confident the problem lies with what you've implemented, especially since the sample in the Storybook works just fine. Maybe there are edge cases relating to enabling the selection feature, or using custom property names, or maybe I'm just missing something about how I'm configuring my component with the Options API.

I'll try to take a closer look soon but since you're planning to work on some extended edge cases and unit testing anyway, you might end up fixing whatever my problem is in the mean time.

from vue-tree.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.