-
-
Notifications
You must be signed in to change notification settings - Fork 26
Handle deferred File Explorer view with a more elegant code #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
base: master
Are you sure you want to change the base?
Conversation
|
Hi @kotaindah55 while I don't have time to examine the code changes, this looks promising based on the description! Indeed, I'd prefer the a more elegant one-off solution (single event and done), instead of the currently employed, also event-driven way of focused listening on changes of a selected DOM node. While from performance perspective there will be no difference, what you propose - based on the description - promises clarity and simplicity of the solution. The current solution, handled by the single function |
|
Thanks for your respond! My logic above also can be considered one-off solution. The handler will only be applied if the File Explorer view entered the deferred state. On the contrary, if the view was fully loaded at the first time, the handler is not needed anymore (since deferred views only happen at startup, the next added view will always be fully loaded). Before I move forward applying the logic, probably you want to look at my codes first (it's not a lot of codes) to check whether it will affect the other logic or not. |
|
Hi @kotaindah55, I'm short on time these days and I'd like to look and review the entire solution, when ready. If you make it as an isolated and reusable module (which probably is your approach), testable standalone and in isolation, this would prove it is a safe alternative, also for other Obsidian plugins. It can be published as a reusable standalone project in github. I noticed, that some plugin authors still struggle with correct handling of the not-so-trivial Obsidian deferred views. Then it will need to be compared with the existing solution in this plugin from various perspectives and we will see. Thanks for contributing! |
|
Hi @SebastianMC! I've finished the refactor, also tested the modified plugin in Obsidian application directly. From the result I got, the plugin works as expected. |
|
Hi @kotaindah55, thanks for the updates! I've briefly examined the changes in While the resulting code could be perfectly valid and working (time consuming tests and detailed code review are needed), this range of changes is too risky at the current stage of the plugin (stable, maintenance, minor improvements and bugfixing). Testing all of the scenarios would require much time. If your goal is to integrate this with the plugin, could you please:
Overall, please keep in mind the big picture:
|
If we look closely at
Viewinstance, we will find thatView, includingDeferredView, is one ofComponentsubclasses (Pluginis also one of them). Therefore, we can register a callback that will be called once theDeferredViewhas been unloaded, being exchanged with the actual view.Progress:
This PR also introduces some changes:
Filesis disabled.setWatcherForDelayedFileExplorerView()anddelayedApplicationOfCustomSorting()are no longer in use.