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

Refresh on save, remove duplicate edge in juggl #328

Merged
merged 4 commits into from
Feb 13, 2022
Merged

Refresh on save, remove duplicate edge in juggl #328

merged 4 commits into from
Feb 13, 2022

Conversation

HananoshikaYomaru
Copy link
Contributor

@HananoshikaYomaru HananoshikaYomaru commented Feb 7, 2022

this pull request solve two issues.

repeated edge in juggl codeblock

As hiererchy note will make real up and real down, juggl will show two edges overlapping with each other.

Add a setting showUpInJuggl : boolean. When this is false, no more repeated edge, only down is shown.

no refresh on save

add a setting refreshOnNoteSave : boolean, when this is check, refresh index on save🤙🏻

bug on adjust hierarchy manipulator when using hierarchy note #330

Hierarchy note show nothing when using hierarchy note folder. The bug is fixed.

Also, enter / click is changed to shift + enter because the purpose of the quick adjust panel is to adjust without going into the hierarchy notes. Therefore, the most common key should map to add child.

Also, the default value should not be <empty>. It should be the file name of active leaf.

TODO

@HEmile HEmile self-requested a review February 7, 2022 09:43
Copy link
Collaborator

@HEmile HEmile left a comment

Choose a reason for hiding this comment

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

I'd recommend splitting up the two commits as they are introducing very different features. What is the motivation for the second commit? @SkepticMystic Maybe the second commit can better be merged as replacing the on-change event as it might be more efficient?

src/Settings/GeneralSettings.ts Outdated Show resolved Hide resolved
src/Settings/MatrixViewSettings.ts Show resolved Hide resolved
src/interfaces.ts Show resolved Hide resolved
@HananoshikaYomaru
Copy link
Contributor Author

HananoshikaYomaru commented Feb 7, 2022

@SkepticMystic @HEmile I also fix the issue on adjust hierarchy manipulator. Please check.

On save is more reasonable than on change file because the breadcrumbs tree technically has no change when I only navigating in my vault. Also, on change file is slow. I didn't replace the on change file event because I am not sure how you guys will think about this. But on save is definitely more reasonable so I added this setting.

@SkepticMystic
Copy link
Owner

@HananoshikaYomaru thank you for all your effort here :)
I've just now had a chance to go over your code, and it looks good. There are a couple things I want to change, but nothing major.
I'm gonna pull this locally to test out, and then I think I'll merge it!

@SkepticMystic
Copy link
Owner

@HananoshikaYomaru the way I ended up merging means the changes won't be credited to you on the Contributors page. I can try do it again to make it show that you made these changes?
But I've already adjusted some of your code, so my changes would be deleted.
Let me know what you think

@HananoshikaYomaru
Copy link
Contributor Author

Thanks for testing for me. I am very happy to contribute to breadcrumbs. I definitely want my contribution show up in the repos. So it might be a bit troublesome but please help me adjust it back.

@SkepticMystic SkepticMystic merged commit 0c8347e into SkepticMystic:master Feb 13, 2022
@SkepticMystic
Copy link
Owner

Cool cool, I think this worked

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.

3 participants