dgreene1 / react-accessible-treeview

A react component that implements the treeview pattern as described by the WAI-ARIA Authoring Practices.
https://dgreene1.github.io/react-accessible-treeview
MIT License
261 stars 37 forks source link

Easy way to have all nodes opened by default #183

Closed Ctrlmonster closed 3 months ago

Ctrlmonster commented 3 months ago

The problem Right now if you have a dynamically changing tree (i.e. inside an editor) where the tree-view often gets re-created from scatch, it becomes unreasonably cumbersome to tell the view to just open all tabs by default (i.e. to view the full content of the new tree). The reason for this is mainly that there is an error thrown if you pass an id that the tree doesn't know about, i.e. Node with id=MY_ID doesn't exist in the tree,

Describe the solution you'd like Either provide a new expandAll prop or allow the user to pass another value to expandedIds that signals the intent of opening up all tabs (i.e. null, true or some symbol)

<TreeView
  expandedIds={null} // pass signal prop to open up all tabs
/>

Describe alternatives you've considered

Attempt 1: dynamically get all ids:

// This doesn't work, leading to above mentioned id doesn't exist error (seems like it shouldn't though)
return (
  <TreeView 
    data={data}
    expandedIds={data.map(({id}) => id)}
  />
)

Attempt 2: pass all possible future ids (only limited usecase, would theoretically work in my case)

// This doesn't work, leading to above mentioned id doesn't exist error (because there are extra/unnecessary ids)
const ids = Array.from({length: 1000}).map((_, i) => i);

return (
  <TreeView 
    data={data}
    expandedIds={ids}
  />
)
dgreene1 commented 3 months ago

@mellis481 can you read through this and give your feeling on it?

@Ctrlmonster how would your proposed prop work when someone closes a branch? Would this prop only be for on first render? If yes, then I think we can open this issue for “help requested” after we rename the prop to be expandAllOnFirstRender. If the answer is no (ie you want everything always open) then I think that would introduce serious accessibility issues (like why would we allow nodes to be closed if it just reopens), then I think this is out of scope for this library given that our goal is primarily to be accessible. We wouldn’t want to lie to screen readers and let them think that they can fold a branch.

mellis481 commented 3 months ago

@Ctrlmonster Using the defaultExpandedIds property works for me using your code where you simply map the id prop of your data. Can you confirm the items in your data array have an id prop?

Ctrlmonster commented 3 months ago

@Ctrlmonster Using the defaultExpandedIds property works for me using your code where you simply map the id prop of your data. Can you confirm the items in your data array have an id prop?

They definitely do. Are there any requirements on the ids or can they just be completely random? If there aren't, this seems to be a problem with my code. But it's really weird as the data gets created inside the component and the ids get mapped a few loc below, not sure how they can't be fresh if both get re-created on component re-render.

@dgreene1 That's a good point, it would really only make sense on the first render (and that's what I need as well), but if expandedIds={data.map(({id}) => id)} should work then I'm not sure if an extra prop is needed.

dgreene1 commented 3 months ago

Okay then we’ll close the issue for now.