search

simonexmachina / jquery-bonsai

Super lightweight jQuery tree plugin
http://simonwade.me/jquery-bonsai
MIT License
148 stars 42 forks source link

Implemented setSelected function #18

Closed dfritschy closed 9 years ago

dfritschy commented 9 years ago

I am suggesting a setSelected function which highlights the currently selected list item and its parents.

The use case is a menu tree which can thus be "statically" built and responds to the currently loaded page using this function.

The list item is identified by its id or data-bonsai-id attribute.

simonexmachina commented 9 years ago

Hi @dfritschy, I did some thinking and have realised that the whole GUID, generated/specifiedIdPrefix business is unnecessary, so I've pushed v2.0.0-beta and I'd really appreciate if you could confirm that it still works for your use case. There's details of the changes in the changelog. If you find anything doesn't work for you then please do let me know.

Regarding this PR, I try to keep bonsai as lean as possible - I think that's one of the reasons why it's such a great little module. So I think that a expandTo(listItem) function is valuable, but I'm reluctant to add a setSelected() function because I think there's app-specific behaviour involved. My philosophy is that the module should provide the underlying primitives, and provide extension points where required to allow app-specific functionality to be easily added.

If you want to rebase this PR against the updated master then I'd be happy to merge an expandTo(listItem) function (sorry that master's moved on under you, but you gotta take the inspiration when it strikes!).

dfritschy commented 9 years ago

Hi @aexmachina, thanks for this constructive dialog! Regarding v2.0.0-beta, I have tested persisting and restoring the state with both id and data-bonsai-id attributes. For our use cases it works fine, as we always will have some identifier for the list elements. Regarding this PR, I fully agree with keeping bonsai lean (I would even suggest moving the checkbox stuff to another module), and I will provide an expandTo(listItem) function (although I doubt mine will be as elegant as your code). We can move the setSelected functionality to our code easily, certainly it would help if expandTo() returns the listItem, so that we can proceed from this one. I see a great potential for bonsai, as I am trying to replace the limited navbar functionality of Bootstrap with a nested, multi-level mobile offcanvas navigation based on bonsai. It's the only plugin I have found which has such a clean architecture and semantics.

dfritschy commented 9 years ago

Hi @aexmachina, sorry for messing up. I obviously did not properly rebase my branch... Only the last commit 1eba42d is relevant.

simonexmachina commented 9 years ago

@dfritschy thanks! That's really good to hear. I'd also like to have the checkbox stuff in another module, but that's how it came out years ago, and I'm comfortable with keeping it there for now.

And yes I agree that expandTo() should return the list item.

I've just completed another few refactorings. Shouldn't affect your work, but I'd like to get confirmation from you. See the CHANGELOG and changes to README. Once you've confirmed it's all good I'll push a new version.