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

permalinks: add a few bits to what's accepted in a URL, populate history events, add a copy to clipboard button #57

Open
wants to merge 3 commits into
base: gh-pages
Choose a base branch
from

Conversation

cdanis
Copy link

@cdanis cdanis commented Aug 14, 2018

Extend permalinks to support deck names and designers.
Add a permalink href on the page, auto-populated by making edits.

index.html Outdated Show resolved Hide resolved
js/decklist/main.js Outdated Show resolved Hide resolved
@chrisajimenez
Copy link

How is this coming along? I'm happy to help out here, the ability to generate a permalink would be awesome.

@cdanis
Copy link
Author

cdanis commented Mar 11, 2019

How is this coming along? I'm happy to help out here, the ability to generate a permalink would be awesome.

Sorry, realized a bunch of this was harder than I expected, then started interviewing for a new job, which I then got (great for everything except this PR ;)

The larger bit of functionality I had wanted to implement -- QR codes that link to a list's permalink, and with said QR code printed on the page, so it's easy to take last week's printout and tweak a few things -- require the use of a link shortener service to get reasonable QR codes. This, of course, brings up a ton of questions: which link shortener? Can decklist.org host its own? Do we need to use an external service that provides an API? If so, how do we manage API keys? etc.

I still want to finish up this PR, because I think permalinks are a useful thing in general, but wasn't sure what the best way was to figure out any of the above stuff...

@chrisajimenez
Copy link

Congratulations on the new job! 🔥 🔥

I think it is great that you split up that work into two parts. For now, I think being able to generate the permalink and save it manually somewhere is very useful. There is a popular library, clipboard.js that allows copying to clipboard, as @Nightfirecat suggested. I'm happy to try and implement that on top of this pr, if you don't mind?

FWIW I've been using your generatePermalink function manually in the console to create the links myself, so that works great!

@Nightfirecat
Copy link
Contributor

I don't know that a library would be needed--copying to clipboard using the native Clipboard API is well-supported among major browsers and likely wouldn't need any polyfills to be achieved.

@cdanis
Copy link
Author

cdanis commented Apr 21, 2019

I believe this is good to go. As requested, it's a button, and I also figured out the History API enough for it to Do The Right Thing on any modification, as far as I can tell.

Demo running at https://decklist.nucleosynth.space/

@cdanis cdanis changed the title part 1 of permalinks: add a few bits to what's accepted in a URL, populate a field on the page permalinks: add a few bits to what's accepted in a URL, populate history events, add a copy to clipboard button Apr 21, 2019
@cdanis
Copy link
Author

cdanis commented May 5, 2019

friendly ping :)

@cdanis
Copy link
Author

cdanis commented Jun 16, 2019

@Nightfirecat @april any chance you'll be able to take a look soon?

Once this is merged I'd like to modify PDF output to include a hyperlink to the permalink URL, which is really useful for 'editing' a PDF you saved the other week. I have this code mostly ready-to-go.

@april
Copy link
Owner

april commented Jun 25, 2019

This looks good, but would you be willing to take out the stuff that isn't related to this specific change, e.g. the parts about pulling down the cards and parsing them?

@cdanis
Copy link
Author

cdanis commented Jun 25, 2019

Are you talking about 81c6883, e8c0353, and ea08763? I think those are appearing here as a result of a merge commit. I don't think they will cause any effect if this is closed as a rebase atop your master branch.

@cdanis
Copy link
Author

cdanis commented Jun 25, 2019

Oh. Hm. Now I see what you're talking about, and I'm not sure how that happened. I'll fix.

@cdanis cdanis force-pushed the upgrade-permalinks branch from 3457a7a to a8c68cf Compare July 11, 2019 01:18
@cdanis
Copy link
Author

cdanis commented Jul 11, 2019

I've rebased this on your current gh-pages and I think it's good to go now.

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.

4 participants