Add table generator

[Quorafind]

I am submitting a new Community Plugin

Repo URL

Link to my plugin: https://github.com/Quorafind/Obsidian-Table-Generator

Release Checklist

• [ ] I have tested the plugin on
  • [x] Windows
  • [ ] macOS
  • [ ] Linux
  • [ ] Android (if applicable)
  • [ ] iOS (if applicable)
• [x] My GitHub release contains all required files
  • [x] main.js
  • [x] manifest.json
  • [x] styles.css (optional)
• [x] 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)
• [x] The id in my manifest.json matches the id in the community-plugins.json file.
• [x] My README.md describes the plugin's purpose and provides clear usage instructions.
• [x] 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.
• [x] I have added a license in the LICENSE file.
• [x] My project respects and is compatible with the original license of any code from other plugins that I'm using. I have given proper attribution to these other projects in my README.md.

[joethei]

this.tableGeneratorEl.style.position = "absolute"; you should move these into the CSS file to make it easier to style, if possible.

SampleSettingTab Rename this to avoid conflicts.

this.applyDebounceTimer = window.setTimeout(() => { You can use the .registerInterval from the API instead, it will be automatically unloaded.

this.registerInterval(window.setTimeout(() => {
            plugin.saveSettings();
        }, 100)
);

---

The following are just style nitpicks, you don't have fix these.

(evt.target as HTMLElement) You are doing this multiple times, at multiple places, you could just save this in a variable.

[Quorafind]

@joethei Fixed in https://github.com/Quorafind/Obsidian-Table-Generator/commit/26476699f14e90a1d4ef4e01b4e577d46ea45dbd

[liamcain]

• target.classList I think you need to check if target is null here too.
• this.registerDomEvent(window, 'click', (evt: MouseEvent) => { I think you don't need this event listener. You should just keep the event listener on document and the other window-open event listener. I think that's enough though.
• if (this.tableGeneratorEl === undefined) return this line is redundant, you can remove it.
• class tableGeneratorSettingTab capitalize the name of your class
• el.style.minWidth = "2.3em"; you should just add a class and do these tweaks with CSS so that themes/snippets can override if they need to.
• defaultWindow?: boolean I don't think you need this flag. Also this function is really hard to read since you have so much on one line. Could you reformat this function? I'm also not sure why you are using both body.contains and querying for the element. Querying for the element should be enough. That said, you have a reference to the table element, so there's no reason you should be querying for it in the document at all. Just use this.tableGeneratorEl.detach()
• $: if (selectedTableEnd) { this looks dangerous. Why are you marking these svelte blocks as reactive? I think you are better off keeping these inside the event callbacks to avoid these triggering too frequently
• export const tableGeneratorMenu I don't see much advantage to having this in a util function. Just put this code directly into your plugin
• tableGeneratorBoard.style.left = ${ coords.left }px; Put these styles in your css file instead

[claremacrae]

I'm sorry not to see this added. I've found it useful via BRAT.

[Quorafind]

Sorry that I didn't update this. And it closed automatically because I pulled a new request. I will reopen this soon. @claremacrae

[claremacrae]

Oh that's great news. Thanks!

[Quorafind]

@liamcain hi, I fixed all these issues.

[liamcain]

• if (!this.tableGeneratorEl) return; Unnecessary check, createElement won't fail, so this will never run
• if (this.tableGeneratorEl) this.tableGeneratorEl.detach(); No need to detach if you're just going to reattach. appendChild will automatically detach if the element is attached elsewhere. I feel like the logic you have here is a bit backwards. I think what you want to do is check if tableGeneratorEl doesn't exist, if so, create the element. Then unconditionally attach the element to the body.
• registerTableGeneratorMenu afaict, this function doesn't do anything. I think you can just remove it.
• style You can use svelte's style directive instead. Another cool option would be passing in the row and column as css vars. That way you can keep the css inside your style tag.
• hideTable You already have a reference to your table instance in the plugin. I think it's better to just pass around the reference to the table rather than querying the DOM for it. Or just have hideTable be a function on the plugin instance, then call plugin.hideTable if you want to avoid passing the htmlelement instance around.

[Quorafind]

@liamcain

if (!this.tableGeneratorEl) return; Unnecessary check, createElement won't fail, so this will never run

Remove. And use createEl from Obsidian now.

if (this.tableGeneratorEl) this.tableGeneratorEl.detach(); No need to detach if you're just going to reattach. appendChild will automatically detach if the element is attached elsewhere. I feel like the logic you have here is a bit backwards. I think what you want to do is check if tableGeneratorEl doesn't exist, if so, create the element. Then unconditionally attach the element to the body.

But appendChild didn't work with popout windows, for example when I create a new table generator in the popout window, the old one would not auto detach. So if you know how to make this work well with popout windows , I need some help 😂 Or I have to leave the code here.

registerTableGeneratorMenu afaict, this function doesn't do anything. I think you can just remove it.

This does same thing like above.

style You can use svelte's style directive instead. Another cool option would be passing in the row and column as css vars. That way you can keep the css inside your style tag.

Used style directive.

hideTable You already have a reference to your table instance in the plugin. I think it's better to just pass around the reference to the table rather than querying the DOM for it. Or just have hideTable be a function on the plugin instance, then call plugin.hideTable if you want to avoid passing the htmlelement instance around.

Move hidetable to plugin class.

[liamcain]

LGTM, do you mind rebasing this PR? Github has decided that I can't resolve the conflicts via the web UI for some reason.

[Quorafind]

@liamcain https://github.com/obsidianmd/obsidian-releases/pull/1212