Skip to content

Add script selection Import/Export feature #453

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

e-salad
Copy link

@e-salad e-salad commented Nov 23, 2024

.

- Improve validation for configuration import
- Refactor SaveSelectionButton.vue implementation
- Add floppy-disk-gear icon for SaveSelectionButton
@undergroundwires undergroundwires force-pushed the master branch 2 times, most recently from 080047d to 2f8aaf4 Compare December 4, 2024 08:01
@undergroundwires undergroundwires force-pushed the master branch 2 times, most recently from 4dd8e11 to 64feb66 Compare December 12, 2024 12:40
@e-salad e-salad marked this pull request as draft December 16, 2024 04:00
@undergroundwires undergroundwires force-pushed the master branch 3 times, most recently from 25d0d37 to e6c52db Compare December 20, 2024 19:29
@e-salad e-salad closed this Jan 13, 2025
@undergroundwires
Copy link
Owner

Hi @e-salad,

I'm sorry for being too late at reviewing this. It's a very good contribution with a clean fix, and I'm really happy to see a good dev looking at it. This is the first time this project has gotten improvement at this depth. And I'm also surprised how cleanly you integrated it, using existing patterns/extending perfectly.

Except some minor styling issues (npm run lint:eslint would fix those), it looks all good.

The only issue is that it will be very painful to handle these files, so based on discussions at #59, I started working on #262. Unfortunately, it took ridiculous long amount of time to get it done due to bugs/other improvements I'm fixing at the side. This is my highest priority now for next feature release. And ID support will change some of the APIs on application layer.

Would you be able to update this PR after that is done? Then instead of persisting the whole state, you'd need to persist the IDs only. And we can iterate with small features/commits on this to have proper backwards compatibility (what happens if something with a specific ID disappears etc.).

@e-salad
Copy link
Author

e-salad commented Jan 24, 2025

Hi @undergroundwires, thank you for the kind words and feedback!

I initially created this PR because I saw the presets ("Standard" and "Strict") as config files, so I thought we could support custom presets in a similar way. However, I closed it as I felt it might be too large of a change to introduce right away. (Apologies for missing previous discussions on this, i searched but couldn’t find them.)

Please let me know how the final ID implementation turns out; I’d be happy to update this PR.

@e-salad e-salad reopened this Jan 24, 2025
@undergroundwires undergroundwires force-pushed the master branch 8 times, most recently from aae488a to 3d98c6d Compare February 1, 2025 12:49
@undergroundwires undergroundwires force-pushed the master branch 3 times, most recently from e3cc404 to bfd1b72 Compare February 6, 2025 11:48
@undergroundwires undergroundwires force-pushed the master branch 3 times, most recently from 298af54 to 07bf5df Compare March 11, 2025 09:36
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.

2 participants