added Advanced Slides

[MSzturc]

I am submitting a new Community Plugin

Repo URL

Link to my plugin: Advanced Slides

Release Checklist

• I have tested the plugin on
  • Windows
  • macOS
  • Linux
• My GitHub release contains all required files
  • main.js
  • manifest.json
  • styles.css (optional)
• GitHub release name matches the exact version number specified in my manifest.json (Note: Use the exact version number, don't include a prefix v)
• The id in my manifest.json matches the id in the community-plugins.json file.
• My README.md describes the plugin's purpose and provides clear usage instructions.
• I have read the tips in https://github.com/obsidianmd/obsidian-releases/blob/master/plugin-review.md and have self-reviewed my plugin to avoid these common pitfalls.
• I have added a license in the LICENSE file.

[nothingislost]

Great plugin idea! A couple notes:

1. You're missing a link to your plugin in the main comment above. You'll want to add https://github.com/MSzturc/obsidian-advanced-slides
2. You're using a branch other than master so you'll want to add a line to your plugin block stating "branch": "main"
   a) See this plugin for an example https://github.com/obsidianmd/obsidian-releases/blob/master/community-plugins.json#L37

[MSzturc]

Added the branch and link.

Thx for the fast feedback!

[MSzturc]

@nothingislost
What's the status here? Is there something I can / must do to move the process forward?

[aidenlx]

Nothing particular, I think. You will have to wait until Licat have time to do code reviews. (I've been in the waiting list for a month or so, btw, so just be patient😂)

[lishid]

https://github.com/MSzturc/obsidian-advanced-slides/blob/efa5b1fd9c29a74f54039a0f921e4b8da9689689/src/main.ts#L38
Obsidian's config dir isn't necessarily .obsidian (it's configurable). You can find it at Vault.configDir.

https://github.com/MSzturc/obsidian-advanced-slides/blob/efa5b1fd9c29a74f54039a0f921e4b8da9689689/src/main.ts#L120
See the tip at: https://github.com/obsidianmd/obsidian-releases/blob/master/plugin-review.md#avoid-managing-references-to-custom-views

https://github.com/MSzturc/obsidian-advanced-slides/blob/efa5b1fd9c29a74f54039a0f921e4b8da9689689/src/revealPreviewView.ts#L19
Recommend using window.open here instead of using an electron dependency.

https://github.com/MSzturc/obsidian-advanced-slides/blob/efa5b1fd9c29a74f54039a0f921e4b8da9689689/src/revealPreviewView.ts#L49
Recommend adding a sandbox directive for iframe elements.
This function also doesn't need to be async.

https://github.com/MSzturc/obsidian-advanced-slides/blob/efa5b1fd9c29a74f54039a0f921e4b8da9689689/src/main.ts#L124
Here you are ignoring the changed file completely - the problem is, the change event could be fired consecutively for many files (for example, renaming a file causing all links to that file to update).

At the very least this function should be debounced. I also recommend only refreshing your view when the file of interest has changed.

[MSzturc]

Thank you for your constructive feedback. I have taken into account all your points and I have already implemented them in my code.

As for the point with debounce. I noticed that obsidian only saves the active document every 3 seconds or so. This means that a debounce has already been implemented there, which is why it does not make sense to implement a second debounce here. What I'm going to do, however, is debounce the rendering process itself, so that the preview gets only rendered once per second. not on every change event.

[lishid]

I noticed that obsidian only saves the active document every 3 seconds or so. This means that a debounce has already been implemented there, which is why it does not make sense to implement a second debounce here.

We do, but only in the editor. Plugins can save files as many time as they wish. Obsidian Sync, for example, may trigger hundreds of change events when another device comes online and syncs all of the remote changes. There are also cases like file renames, which can trigger a link update on dozens of files at the same time. You're also not filtering for file changes that aren't related to your current file, which would make your code refresh more often than necessary.

Either way, since you already debounce the rendering, it's already a massive improvement. The rest is just a nit.

https://github.com/MSzturc/obsidian-advanced-slides/blob/0f3dc28530fc56e7da86330cca0c3298ff99c2d9/src/obsidianUtils.ts#L36
Here you might want to consider using metadataCache.getFirstLinkpathDest (which uses a unique filename lookup table rather than iterating through all files in the vault) (similar to the advice here: https://github.com/obsidianmd/obsidian-releases/blob/master/plugin-review.md#avoid-iterating-all-files-to-find-a-file-by-its-path)