Seemingly outdated info in Plugins/User Interface/Views
senritsu opened this issue · 0 comments
There are a few pieces of information below the code examples in Views.md that do not match the code and reference apparently outdated functions.
If I get the required clarification and feedback on the additional proposed change, I will open a PR.
Cleaning up a view
The docs mention
In the
onunload()
method, to make sure that you clean up the view whenever the plugin is disabled:
- Allow the view clean up after itself by calling
close()
.- Detach all leaves that are using the view.
Ignoring the minor grammar error of the missing "to" in "to clean up" for now, I can't figure out what the referenced close()
function is supposed to be. There is no View.close
, just View.onClose
, but as that is a lifecycle event/method, I would assume that is called by Obsidian internally already.
Is it enough to just detach the leaf? If not, what is the correct way to properly and fully clean up a view?
Singleton view instance vs. multiple allowed instances
The docs mention in the tip at the end of Views.md
Try commenting out the call to
detachLeavesOfType()
to allow the user to create more than one leaf.
This is confusing as there is no detachLeavesOfType()
call in the example. This seems to be a leftover from 5ca01f6, before which the view was destroyed and recreated. The current example does it a bit differently and checks if the view already exists before creating it, so the function call is gone.
For my own plugin experiment I refactored the example code a little bit, to:
export default class ExamplePlugin extends Plugin {
// ... rest of the class
async onunload() {
this.findView()?.detach();
}
findView() {
for (const leaf of this.app.workspace.getLeavesOfType(VIEW_TYPE_EXAMPLE)) {
if (leaf.view instanceof ExampleView) {
return leaf;
}
}
}
async createView() {
const leaf = this.app.workspace.getLeftLeaf(false);
await leaf.setViewState({ type: VIEW_TYPE_EXAMPLE, active: true });
return leaf;
}
async activateView() {
const leaf = this.findView() ?? (await this.createView());
this.app.workspace.revealLeaf(leaf);
}
}
If this is seen as an improvement I could adjust both the example and the tip, as this approach makes it easy to substitute const leaf = this.findView() ?? (await this.createView())
for const leaf = this.createView()
to switch from singleton-view to multi-view. I am open to discussing the method names though, as I am not completely content with having findView
actually return a leaf and not a View
. On the other hand, having a short and clear name for what the method conceptually does is nice too.
Otherwise, I can see if I can change the section to match the current example code. As it would involve removing the conditional blocks and just executing the contents of the else
, it should be possible to write a concise and clear replacement for the outdated tip.