Skip to content

Add prop keySeparator#186

Closed
PRGfx wants to merge 2 commits into
iannbing:masterfrom
PRGfx:task/keySeparator
Closed

Add prop keySeparator#186
PRGfx wants to merge 2 commits into
iannbing:masterfrom
PRGfx:task/keySeparator

Conversation

@PRGfx
Copy link
Copy Markdown

@PRGfx PRGfx commented Nov 17, 2020

Introduce the option to split/join node paths by a character other than /.

It seems I had to go through the walk setup, which led to a lot of changes in the tests.
I'm not convinced about defining the default separator in two places, but I am not sure where best to define such "shared" value.

Resolves #162

Introduce the option to split/join node paths by a char other than `/`.
Comment on lines +98 to +100
data={mockDataWithSlashes}
openNodes={['item2/a']}
keySeparator=";"
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @PRGfx , thanks for the PR! It's indeed a useful prop!
Is it possible to make this test case more specific by setting openNodes to item2/a;b (and b needs nodes as well)? So that it can fully demonstrate the use case of this new prop 🙂

Copy link
Copy Markdown
Owner

@iannbing iannbing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PRGfx Thanks for the PR!
Could you improve the test case? I'd be happy to merge it.

@PRGfx
Copy link
Copy Markdown
Author

PRGfx commented Jan 10, 2021

Hey, sorry for the delay, I don't really need this feature. I adjusted the test-case as you suggested. However openNodes needs to be ['item2/a', 'item2/a;b'] - I would have expected that it would open this path on its own, but that's for another topic.

@iannbing
Copy link
Copy Markdown
Owner

Closing this as superseded by the v2 rewrite.

The keySeparator feature shipped as part of v2.0.0 — currently published as react-simple-tree-menu@2.0.0-rc.2 on npm's next tag. Implementation in PR #220, which bundled it with the broader rewrite:

  • keySeparator?: string on <TreeMenu> (default "/") — threads through everywhere the library touches path keys: emitted Item.key paths in walk(), openNodes lookups, LEFT-arrow parent-focus navigation, and a matching optional second arg on unflatten(items, keySeparator?).
  • New public helper collectBranchKeys(data, keySeparator?) for consumers doing controlled openNodes who want an expand-all that respects the same separator.

Thanks for the original proposal and for flagging issue #162 — the design conversation here helped shape the final prop.

Try the RC:

```
npm install react-simple-tree-menu@next
```

@iannbing iannbing closed this Apr 21, 2026
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.

Key with '/' causing issues

2 participants