Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tree View #93

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

johngeorgewright
Copy link
Contributor

@johngeorgewright johngeorgewright commented Oct 7, 2016

1st of all... this started off as more of a POC which exgallated somewhat. Feel free to use a much or little of this as you like.

This is what was going through my head:

There needs to be different behaviours for jumpy depending on what we're jumping to. As I could see the potential for this logic to grow I first separated all the "label logic" (label creation, destruction, filtering, selection, beaconing etc) in to a separate module. Then it became quickly apparent that this module was simply going to be an iterator (of sorts) that would abstract different label behaviours. I've called these "label-managers".

An instance of label-manager-iterator is set as a property of the jumpy-view and whenever some kind of "label" function needs to happen, the jumpy-view calls a method on the label-manager-iterator.

The label-manager-iterator does exactly what it says on the tin; calls the same method on all label-manager's. There's currently one for text editors and one for the tree view.

That's essentially it.

There are no tests yet. As I said... it's a POC we can bosh out some tests if/when a design is laid out. I didn't want to make any tests if this was going to be chucked.

jumpy-tree-view

See #70

@DavidLGoldberg
Copy link
Owner

This is so awesome. I'll try and get some time this weekend to play with it!

@DavidLGoldberg
Copy link
Owner

BTW, you're a beast. Fast as heck and saw some slick stuff in there. I'll have to read it more this weekend.

@johngeorgewright
Copy link
Contributor Author

johngeorgewright commented Oct 18, 2016

Just learnt something about using setTimeout within atom code:

https://github.com/atom/atom/blob/master/spec/spec-helper.coffee#L186

😭 took me a while to figure out that clocked functions are spies within tests. You need to use the advanceClock() function to make clocked functions work!

@DavidLGoldberg
Copy link
Owner

Yeah, every time I work with that timing stuff.....yuck heh.

@johngeorgewright
Copy link
Contributor Author

Finally got all the tests working :) Pretty much ready for a review now.

@DavidLGoldberg
Copy link
Owner

Awesome job man, I have a few things I actually kind of semi emergency want to fix in Qolor (my other package) I'm not happy with the way something went in that...regression ish. I'll do that soon and if / when stuck I'll be attacking this.

Also, I've been thinking of some new ideas of how to architect ...well everything ... a little bit more like reactive x, for work, qolor, and of course jumpy...

for example... that whole section where you do the unless's for example would be cleaned up a lot.

BTW, I know "unless" is part of coffee and a bunch of the cool kids do it. I hate it. If I don't clean up that whole part with this PR, I'll probably undo that one commit (the stuff in the "syntactical sugar" commit from above ^. Hope you don't mind :\

Anyway, like I said I have some crazy ideas so we'll see. I'll definitely be attacking all of this soon, and I've been reading along with the commits so some ideas have been brewing.

@johngeorgewright
Copy link
Contributor Author

Cool. No worries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants