Skip to content

Conversation

@caasion
Copy link
Collaborator

@caasion caasion commented Aug 17, 2025

Refactor Settings to Svelte

@caasion caasion changed the title Svelte refactor: settings to svelte Aug 17, 2025
@caasion caasion marked this pull request as ready for review August 17, 2025 13:09
@caasion caasion requested a review from aptend August 17, 2025 13:09
@caasion
Copy link
Collaborator Author

caasion commented Aug 17, 2025

Hello,

Please review the implementation of the settings with svelte.

I simplified the saving process a lot (I didn't use the state object, but I directly mutate the settings instead), but this might also cause performance issues now, so please let me know if you want me to add that.

@caasion
Copy link
Collaborator Author

caasion commented Aug 17, 2025

related to #59

@aptend
Copy link
Owner

aptend commented Aug 18, 2025

Long time no see! Great, great, great! It looks like you are more confident in coding! It is a big change, I will take a look, and learn Svelte…… again 😅

@caasion
Copy link
Collaborator Author

caasion commented Aug 18, 2025

Yes! It's nice to see you still working on this plugin. And yes I finally set out to follow a course in coding, so I'm able to actually understand what I am doing now 🤣🤣.

Note that I am using svelte 5 runes mode for the settings (which is often different from "legacy mode"). The svelte docs are up to date, but sometimes discussions of svelte aren't updated yet, so watch out for that.

@caasion
Copy link
Collaborator Author

caasion commented Aug 18, 2025

Ah, also, I'd like to add support for having multiple profiles active at once. I didn't do that yet because I wanted to keep the refactoring as close to what you've done so far.

Also, for storing profiles, maybe we can use a Record data structure since that is the shape of objects in json anyways. Let me know what you think!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants