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

Add ability to download config #69

Merged
merged 3 commits into from
Apr 27, 2023
Merged

Conversation

GeraldTM
Copy link
Contributor

@GeraldTM GeraldTM commented Apr 19, 2023

Generates a zip of the generated json config in the correct file structure, and serves it to the user. IMO a Useful QOL Change that eliminates more of the configuration headache.

@GeraldTM
Copy link
Contributor Author

@thenetworkgrinch I think this is done, if you want to check it out, that'd be great.

@thenetworkgrinch
Copy link
Contributor

thenetworkgrinch commented Apr 20, 2023

Thank you @GeraldTM! I am at worlds right now and will review it on monday. I appreciate this PR (and you managed to snipe #69 😂)

@GeraldTM
Copy link
Contributor Author

Oh! Glhf then, hope it goes well!

@thenetworkgrinch
Copy link
Contributor

Finally had a chance to review this, sorry for taking so long. I love it! The only thing I wonder if it is necessary is the () in onclick which is semantic at best and not really a concern. I love how you used the namespace to bypass popup blockers too!

@thenetworkgrinch thenetworkgrinch merged commit 50bff82 into BroncBotz3481:main Apr 27, 2023
@thenetworkgrinch
Copy link
Contributor

The only other semantic things I will fix at a later date are you usage of semicolon's but I dont think I was consistent with those either so it is a problem for later.

@GeraldTM
Copy link
Contributor Author

Lol idk about all that, I'm just getting into web dev, coming from Java python and c# and thought I'd give this a go (google is my best friend). Also I didn't write the saveAs function, it's credited above it as a drop in replacement for a library that wasn't working.

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