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

(Bug report) Custom note-detail-pane widgets are not reinstantiated #4274

Closed
rauenzi opened this issue Sep 21, 2023 · 1 comment
Closed

(Bug report) Custom note-detail-pane widgets are not reinstantiated #4274

rauenzi opened this issue Sep 21, 2023 · 1 comment

Comments

@rauenzi
Copy link
Collaborator

rauenzi commented Sep 21, 2023

Trilium Version

0.60.4

What operating system are you using?

Windows

What is your setup?

Local + server sync

Operating System Version

Windows 10 v1909

Description

I noticed an issue with my Breadcrumbs widget breaking on any tab/split. It seemed to only update the most recently opened pane. So I did some digging... It seems because SplitNoteContainer uses a widgetFactory (https://github.com/zadam/trilium/blob/master/src/public/app/layouts/desktop_layout.js#L120) all the widgets inside that function have separate instantiations per split/tab except for the note-detail-pane custom widgets since they are expected to export an instance of the widget. This would mean that these widgets are "attached" to the latest split/tab only and previous ones are left behind which matches what I was experiencing with my widget.

To test this out locally I modified the get of WidgetsByParent (https://github.com/zadam/trilium/blob/master/src/public/app/services/bundle.js#L55) to look more like this:

    get(parentName) {
-        return this.byParent[parentName] || [];
+        if (!this.byParent[parentName]) return [];
+        return this.byParent[parentName].map(w => w.prototype ? new w() : w);
    }

So basically checking if the exported widget was actually the full class with prototype as opposed to an instance and instantiating it if needed that way each call of the widget factory got a new instance.

Then in my widget I just made the parentWidget getter be static so it would pass the bundler's check. Then only exported the class itself instead of an instance.

const position = api.startNote.getLabelValue("breadcrumbsPosition") ?? "";

class BreadcrumbWidget extends api.NoteContextAwareWidget {
    get position() {return position === "bottom" ? 100 : position === "top" ? 1 : 50;}
-    get parentWidget() {return (this.position === 100 || this.position === 1) ? "center-pane" : "note-detail-pane";}
+    static get parentWidget() {return (position === "bottom" || position === "top") ? "center-pane" : "note-detail-pane";}
    ... // rest of the widget
}
-module.exports = new BreadcrumbWidget();
+module.exports = BreadcrumbWidget;

And this worked out perfectly and allowed the separate instances of the widget to function independently and correctly on each note/split/tab.

I'm sure there are better ways of fixing this, but this was the first that came to mind as proof-of-concept that was still backward-compatible.

Error logs

No response

@zadam
Copy link
Owner

zadam commented Sep 25, 2023

Hmm, yeah ... this was designed before there were splits and thus before the possibility of having two note contexts visible at the same time. 

I applied your patch and updated the wiki. The changes are backwards compatible, just the recommended style for new widgets changes so that they work better with splits.

Thanks.

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

No branches or pull requests

2 participants