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

Remove JS Styling, Minify CSS, Speed Up Settings Page Loading, Remove Use of innerHTML, and Remove Default Hotkey #1190

Merged
merged 3 commits into from
Oct 11, 2024

Conversation

pjkaufman
Copy link
Collaborator

Relates to #1187

Remove JS Styling

As a part of the Obsidian guidelines for plugins, there is a recommendation to not assign styles via the JS. This is because it makes it so that themes cannot style your plugin if they so choose then. So the last couple vestiges of this have been removed.

Minify CSS

As a part of the desire to speed up the Linter and its load times, I decided to minify the CSS that is used. This can improve loading times, but it may not be noticeable on higher end devices since the load time is already probably at a size that the CSS loading even a smidge faster will not be seen. But it is worth adding in my book. I also had it automatically added to the zip file and release on release to make it easier to use in this way.

Speed Up Settings Page Load

Ever since I started working with the Linter's settings on multiple tabs and having them dynamically be styled, the Linter's settings has always felt like it loaded slowly. This was especial true for my phone. So I finally decided to get to the bottom of this.

When I looked into it, it came out to be caused by using the markdownRenderer.render function. It would parse the string looking for markdown and convert it to html. This is not bad when only used where needed, but I was using it everywhere and that added up. So it was taking almost a second to generate the display for the settings on my computer. So I went about removing it. This brought the generation time down to about .1 seconds. This made the settings page feel snappy again.

This did require that markdown in the localization files be converted to html to allow for things to work in a faster manner.

A big thank you to @mnaoumov for his help with getting a secure and fast way in place for adding html strings to the DOM.

Remove Use of innerHTML

There were a couple of places that used this method for adding the html strings to the actual DOM. However swapping to manually creating some elements and using the method for speeding up the settings page load ended up taking care of this. innerHTML is not known for being very secure when adding values even if it does seem like the simplest approach. So removing it is a win for both performance and security as well.

Remove Default Hotkey

The Linter had a single default hotkey that was present and was likely not being used, so I have removed it to not only conform with the Obsidian plugin guidelines, but also to allow users to decide what hotkeys they want.

@pjkaufman pjkaufman self-assigned this Oct 11, 2024
@pjkaufman pjkaufman marked this pull request as ready for review October 11, 2024 21:25
@pjkaufman pjkaufman added dependencies Pull requests that update a dependency file obsidian Obsidian specific syntax issue or feature github labels Oct 11, 2024
@pjkaufman pjkaufman merged commit e27f566 into platers:master Oct 11, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file github obsidian Obsidian specific syntax issue or feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant