-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add plugin: PopKit #3688
Add plugin: PopKit #3688
Conversation
Thank you for your submission, an automated scan of your plugin code's revealed the following issues: Required(skipped) Optional[1]:We recommend against providing a default hotkey when possible. The reason being that it's easy to pick a hotkey that a user already has configured and they could get confused when that key doesn't do what they expect. Also, it's hard choosing a safe default hotkey that's available for all operating systems. Do NOT open a new PR for re-validation. |
/skip innerHTML is used in this code, just translating markdown to html. It's unrelated to the content on the Obsidian page and poses no security risks. const div = createDiv();
await MarkdownRenderer.render(
app,
selection,
div,
'',
new MarkdownRenderChild(div),
);
const html = div.innerHTML;
div.remove(); |
Hi @joethei, when I skipped the automatic checks, the GitHub Actions command to assign a reviewer encountered an error, preventing it from automatically requesting your review for this PR. |
id: "popkit-open-popover",, hotkeys: [{ modifiers: ["Mod"], key: "." }], handlerString: |
@joethei Thank you for your review comments. I have already fixed those issues. I have one issue I'd like to discuss. In my code, I've designed something called handlerString that receives a string and executes it as code (I've removed the related references for now). The goal is to enable users of this plugin to freely add their own custom tools in the future, rather than only using the built-in tools I've written or submitting MRs to my repository. This definitely introduces some security risks, but I'm not sure if there's a better approach. |
@joethei Could you help review again? |
@joethei Could you help review again? |
@joethei Could you help review again? |
help: 'Open obsidian help',, help: '打开 Obsidian 帮助', customTitle: 'Add Custom Action',, plugin: 'Source Plugin: ', import dayjs from 'dayjs'; |
@joethei Thanks for reviewing the code. I have fixed those issues. |
@joethei Could you help review again? |
I am submitting a new Community Plugin
Repo URL
Link to my plugin: https://github.com/zhouhua/obsidian-popkit
Release Checklist
main.js
manifest.json
styles.css
(optional)v
)id
in mymanifest.json
matches theid
in thecommunity-plugins.json
file.I have given proper attribution to these other projects in my
README.md
.