chore: 🤖 prompt users for internet archive URL

[PeculiarE]

Fixes #1011

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

• PR is descriptively titled 📑 and links the original issue above 🔗
• tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with grunt test
• code is in uniquely-named feature branch and has no merge conflicts 📁
• screenshots/GIFs are attached 📎 in case of UI updates
• @mention the original creator of the issue in a comment below for help or for a review

[welcome]

Thanks for opening this pull request! Dangerbot will test out your code and reply in a bit with some pointers and requests.
There may be some errors, but don't worry! We're here to help! 👍🎉😄

[gitpod-io]

[jywarren]

This looks great, and it's a great place to begin. 🎉 I think the dist file may have been added by accident? Shall we keep this PR as just the HTML file?

[PeculiarE]

Thank you!

So, here's the thing. When I saw the dist file as part of my changes in my code editor, I was a bit puzzled because I believed that the contents of the dist folder are not usually tracked on Github.

But then I checked the .gitignore file and saw that this particular dist file is one of two files in the dist folder that are actually being tracked:

Leaflet.DistortableImage/.gitignore

Lines 8 to 10 in d6a9098

	dist/*
	!dist/leaflet.distortableimage.css
	!dist/leaflet.distortableimage.js

And even the pre-commit hook adds it as part of the files to be committed:

Leaflet.DistortableImage/package.json

Lines 87 to 93 in d6a9098

	},
	"husky": {
	"hooks": {
	"pre-commit": "npm run linter && npm run build && git add dist/leaflet.distortableimage.js"
	}
	}
	}

So I'm a bit confused. Are these errors/oversights? Or am I missing something?

[PeculiarE]

Update: If both dist files are truly meant to be tracked, then in what instances should you actually let them be included in your commit or PR?

[jywarren]

Yes, we track dist files because we want them included in our npm package for direct inclusion in a web page, so users of our libraries don't have to run an extra "build" step before using them, and so we can guarantee we've tested the built dist file and it works. Not all projects do that, but maybe more should? But i see pros and cons 😄

Here, I meant, since you haven't made any changes to the src folder, and no npm modules have updated, there shouldn't be a new dist file generated. However, there may be an exception because we've merged a number of Dependabot PRs, which have updated our node modules, some of which are built/baked into our dist files. So maybe that's what's happening? Regardless, since your project doesn't directly deal with those changes, I think it's simpler to not include the dist files since they aren't caused by anything you did. But if this is a simple PR we'll close quickly, and the code seems to work (in the existing examples), maybe it's just OK and we can include the dist files, since occasionally we do need to update our dist files with all the updated npm modules we've merged in.

Usually this would just happen when there are more regular JS updates to the src folder, and recent npm modules would just get swept up with those. But we haven't changed much in src recently.

Maybe a longer term idea would be to try to figure out a script to regenerate the dist files for each dependabot PR. But it sounds complicated! 😅

So, your call - we can merge this if you're pretty sure these are just changes from dependabot updates, or we can exclude the dist files. Thanks for checking!!!

[PeculiarE]

Thanks for the detailed explanation, Jeff! It's way more clear to me now. And nope, I'm not 100% sure the changes are from dependabot 😅, so I've gone ahead to exclude the file from the PR.

[PeculiarE]

I would love to push the popup modal we talked about to this PR as well before we merge it. Thank you 😄!

[jywarren]

OK excellent, let's wait on the modal then. Let me know!

[PeculiarE]

Hi @jywarren!

The pop-up modal is ready, and I've pushed the latest changes.

Here's a brief snippet of how it looks like:

pop-up-modal.mp4

[PeculiarE]

Oh, and I tried to convert one of the tasks on the checklist at #998 to an issue so I could link this PR to it, but it seems I do not have 'write' permissions on this repo. Can that be granted please @jywarren? Thank you!

[jywarren]

Look great! I added your permissions too. Thank you!!! ✨✨✨

[welcome]

Congrats on merging your first pull request! 🙌🎉⚡️
Your code will likely be published to https://mapknitter.org in the next few days.
In the meantime, can you tell us your Twitter handle so we can thank you properly?
Now that you've completed this, you can help someone else take their first step!
See: Public Lab's coding community!

[PeculiarE]

Thank you!!! I have now been able to create an issue for this from the list and link both together.

However, the PR title and the consequent merge commit message on the main branch don't reflect what the commit actually does. I've changed the title but since the merge has happened already, the commit message still remains the same.

Was wondering (if it's not too much trouble) if you could help amend the message to match with the PR's title? I saw a Stack Overflow post on how to do this (https://stackoverflow.com/questions/7279196/git-how-to-edit-reword-a-merge-commits-message) but since it's the main branch, I can't do it myself.

[jywarren]

I'll try resolving this today! Should be simple since no other commits have been made and there are few or no other active contributors right now, otherwise we'd have to warn people who might've pulled the latest commit.

…

On Tue, Aug 16, 2022, 11:49 AM Peculiar Erhisohwode < ***@***.***> wrote: Thank you!!! I have now been able to create an issue for this from the list and link both together. However, the PR title and the consequent merge commit message on the main branch don't reflect what the commit actually does. I've changed the title but since the merge has happened already, the commit message still remains the same. Was wondering (if it's not too much trouble) if you could help amend the message to match with the PR's title? I saw a step-by-step tutorial in Github to do this (https://archive.org/details/texas-barnraising/) but since it's the main branch, I can't do it myself. — Reply to this email directly, view it on GitHub <#1009 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J6RK47D7A5HAN6UUJLVZOZ7RANCNFSM56F5X7FA> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[jywarren]

OK, done! Yeah, typically this can cause quite a bit of disruption if there are other users pulling regularly, but here it seems OK. I had to unlock the protection rules for a moment to do it, it wouldn't even allow me!