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

Add option to change neighbour distance in buffer view #37

Merged
merged 3 commits into from
Jun 20, 2020

Conversation

wohanley
Copy link
Contributor

@wohanley wohanley commented Jun 13, 2020

There's probably some terminology I'm missing here, but these changes make it possible to set the "degree" of connectedness for including nodes in the buffer view. Here's an example at degree 2 (including neighbours of neighbours), the root node being "Network Accountability for the Domestic Intelligence Apparatus":

distance

This is WIP because there's some cosmetic work that isn't done yet, I want to nail down the language, and, most importantly, I want to ask: Is this a feature you'd like to add?

@goktug97
Copy link
Member

Hey, this looks awesome! Other than the cosmetics (as you've mentioned) I think everything is good to go.

@JeffreyBenjaminBrown
Copy link

It seems useful for the edges to have arrowheads. If you're looking just one generation out, that's not necessary, but with more than one generation and it becomes possible for a child to link to one of its siblings.

@goktug97
Copy link
Member

goktug97 commented Jun 14, 2020

;(cons 'arrows "to")
)
(cdr (elt graph 1)))))
(dolist (edge edges-cites)
(let* ((title-source (org-roam--path-to-slug (elt edge 0)))
(title-target (org-roam--path-to-slug (elt edge 1))))
(push (list (cons 'from title-source)
(cons 'to title-target)
;(cons 'arrows "to")

@JeffreyBenjaminBrown
You can enable arrowheads if you want

@JeffreyBenjaminBrown
Copy link

So far I do everything in text, but if I ever need it, I'll remember your invitation, thanks :)

@wohanley
Copy link
Contributor Author

OK, looks like "distance" is the graph theory way to say this, so that's what I'm going with. Here's the current state of the UI in dark mode (the colours are untouched in light mode):

distance-ui

Good enough for my purposes, but I'm happy to change it if there are any strong opinions.

@wohanley wohanley changed the title WIP: Add option to change neighbour distance in buffer view Add option to change neighbour distance in buffer view Jun 15, 2020
@wohanley wohanley changed the title Add option to change neighbour distance in buffer view WIP: Add option to change neighbour distance in buffer view Jun 17, 2020
@wohanley
Copy link
Contributor Author

Actually, I've spotted a bug in this:

oops

I think the easy way to fix this is also fairly sweeping - I'd like to get rid of the size calculations in JS and do that layout in CSS instead. Is there complexity I'm missing that makes that impossible, or another reason that you'd prefer I didn't do that? I can make it work with the existing approach too, so no worries, I'd just have to work my lazy brain a bit harder.

@goktug97
Copy link
Member

I haven’t tested your code yet but I was suspecting this would happen. I didn’t know JavaScript, HTML, CSS before this project and I think I still don’t :D I had to make locations and sizes dynamically change depending on the window size and first thing came up to my mind was this. You can put distance input directly in to the middle if you want, would save you from a lot of trouble.

Can you also rename darkmode in the css to darkmode-distance

select2 still prefers inline styles for some things, so this isn't total
@wohanley
Copy link
Contributor Author

It's no trouble at all, that's what I mean - this way is actually easier for me. It was easier just to write the code than to explain my thinking more, so here's a couple of commits for discussion. The first one moves the dropdown sizing code into CSS, which should be a lot less vulnerable to careless coders like myself. The second shows why I called it darkmode rather than darkmode-distance: it's intended to apply to the entire page, not just the distance controls. I used it here to control the colours of the select2 dropdowns too, for example. How does this look to you?

@goktug97
Copy link
Member

To be honest it looks more cleaner this way. I tried to make it with CSS but CSS was hard to wield :D I will test everything when I got time. Do you still working on the PR or is it finished?

@wohanley
Copy link
Contributor Author

If it looks good to you, it's good by me.

@wohanley wohanley changed the title WIP: Add option to change neighbour distance in buffer view Add option to change neighbour distance in buffer view Jun 17, 2020
@goktug97 goktug97 merged commit 0d29fd6 into org-roam:master Jun 20, 2020
@goktug97
Copy link
Member

Sorry for the late review, I didn't have time.

@wohanley
Copy link
Contributor Author

No worries! Thanks!

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.

None yet

3 participants