-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
feat: Allow showing/suppressing the Auto-Suggest in custom editor implementations #2778
Conversation
Thank you for this. I hope to be able to test this out and review it in around a week or so. |
Hi @mgmeyers , we reviewed the changes with @claremacrae today and this seems awesome. Could you please add some documentation so that other potential users of this API could find and use it as well? An 'Auto-Suggest integration' level 2 heading at the end of docs/Advanced/Tasks API would be great (https://publish.obsidian.md/tasks/Advanced/Tasks+Api#Tasks+API) |
Ok, added some docs. Let me know what you think. |
Before this is merged please can the PR title be edited to reflect the now more general facility? |
cursor: EditorPosition, | ||
lineHasGlobalFilter: boolean, | ||
): boolean | undefined { | ||
return (editor as any)?.editorComponent?.showTasksPluginAutoSuggest?.(cursor, editor, lineHasGlobalFilter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mgmeyers just a comment here for my knowledge. I'm curious why the editorComponent
is needed? In Kanban plugin code (https://github.com/mgmeyers/obsidian-kanban/blob/5fa792b9c2157390fe493f0feed6f0bc9be72910/src/components/Editor/MarkdownEditor.tsx#L100-L106) I don't see that being used, what am I missing here? Why not calling just (editor as any)?.showTasksPluginAutoSuggest?.(cursor, editor, lineHasGlobalFilter);
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that's bad naming on my part 😆. The Editor
I have in the kanban plugin is actually an EditorComponent
.
The undocumented API for obsidian's Editor
that gets passed into the suggest is:
class Editor {
editorComponent: EditorComponent;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, so this is Kanban-specific thing? Could you please render that more generic so that other plugins would use this too in Obsidian way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure that this implementation works and this is not my concern. What I'd like to be sure about here is interoperability with Obsidian API that would permit this PR to scale to other plugins as well =) Could you please help me understand if this is the case or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this isn't specific to Kanban, though Kanban is the first community plugin to implement a custom editor. It's important to note, though, that implementing a custom editor requires extending undocumented functionality from Obsidian.
Obsidian has two classes relevant to this discussion: Editor
and EditorComponent
. The Editor
class has an EditorComponent
as a property:
class Editor {
editorComponent: EditorComponent;
}
EditorComponent
houses the Codemirror editor instance whereas Editor
is an abstraction that provides various helper functions for interacting with Codemirror. The EditorComponent
class is not exported through Obsidian's public API, but Editor
is.
Generally, Obsidian passes around the Editor
class instead of EditorComponent
. For example, Obsidian's Editor
class is passed in as the second parameter to EditorSuggestor.onTrigger()
.
The Kanban editor extends the EditorComponent
class and therefore lives under editor.editorComponent
by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To respond to your second comment. This could indeed scale to other plugins. Perhaps the risk here, though, is that it relies on undocumented functionality and therefore could stop working at some point if Obsidian decides to change the internal API. The worst case scenario is that editorIsRequestingSuggest
would always return undefined
and require updating to match internal API changes. It would make sense to me if this feels too much of a risk, and I would be happy to revert this PR in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty ok to leave this and not revert back since it benefits Kanban plugin (Which I didn't use before because it was not supporting Tasks).
But I'd like to explore how this whole ecosystem works to provide better APIs to other plugins like Kanban. You seem to understand this part quite well, would you care to have a live discussion about this, just the 2 of us? I could write you a mail mentioned on the site from your profile. Please let me know =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'd be happy to chat!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oks, sent you a mail)
Just released in Tasks 7.2.0 - many thanks! |
Description
Kanban v2-beta now uses obsidian's native markdown editor. This means that getting tasks working in Kanban cards is fairly easy. This PR serves as an example of how this could be done. I'm not familiar with this code base, however, so not sure if this is the best way to implement Kanban support. Feel free to close this an implement in a different way if desired.
Motivation and Context
Folks have asked for the Kanban plugin to support tasks for a long time now, but it was challenging in the past due to the custom markdown editor that was used. I tried to add support 100% in the Kanban plugin, but there are some major hurdles to doing so. Specifically, kanban does not supply the tasklist formatting to the editor so the editor suggest thinks its in a plain markdown string.
How has this been tested?
I've run
yarn test
. I also tested manually using Kanban version 2.0.18-beta which adds theisKanbanEditor
flag to the Kanban editor.Screenshots (if appropriate)
Types of changes
Changes visible to users:
fix
- non-breaking change which fixes an issue)feat
- non-breaking change which adds functionality)feat!!
orfix!!
- fix or feature that would cause existing functionality to not work as expected)docs
- improvements to any documentation content for users)vault
- improvements to the Tasks-Demo sample vault)contrib
- any improvements to documentation content for contributors - see Contributing to Tasks)Internal changes:
refactor
- non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)test
- additions and improvements to unit tests and the smoke tests)chore
- examples include GitHub Actions, issue templates)Checklist
yarn run lint
.Terms